diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-03-marketplace-update-wrong-package-path-after-drop.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-03-marketplace-update-wrong-package-path-after-drop.json new file mode 100644 index 0000000000..49483e3518 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-03-marketplace-update-wrong-package-path-after-drop.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-11-03", "symptom": "ako/mxcli#1934 (mendixlabs/mxcli#1732): `marketplace update 170 --to 11.5.1 --force` (Community Commons) failed with \"install the new version's bundled files: open package target.mpk: … no such file or directory\" after it had already dropped the installed module and copied units: the project was left half-updated (245 files deleted in mprcontents/) and had to be restored from git.", "cause": "Two defects. The caller derived the package path itself as work/target.mpk, while referenceFor(…, \"t\") downloads to work/t.mpk — so every update looked for a file that never existed. And PerformUpdate read the package only after `drop module`, TransplantModule, identity and grant restore, so the missing input surfaced after the destructive steps.", "file": "cmd/mxcli/cmd_marketplace_diff.go (referenceFor returns the package path it downloaded to), cmd/mxcli/cmd_marketplace_update.go (uses it; no 'mid-update' warning when nothing was written), cmd/mxcli/marketplace/update.go (preflightUpdate before the drop: reference project opens and holds the module, package opens; ErrUpdateNotStarted)", "insight": "A destructive multi-step operation must read every input it will need before the first write — the order, not just the input, was the bug: with the path fixed, a corrupt download or a reference without the module would still have stranded the project. And a path two functions agree on by convention ('the slot letter plus .mpk') belongs to one of them: returning it from referenceFor makes the mismatch impossible rather than corrected. The test hashes every file of a fixture project before and after PerformUpdate with a missing package / reference project; on main both cases changed the project (the reported 'open package' error after the drop), with the preflight stubbed they fail again. The full command needs the marketplace (a PAT) and mx, so it was not driven end to end here.", "refs": ["ako/mxcli#1934", "mendixlabs/mxcli#1732", "ako/mxcli#641"], "test": "cmd/mxcli/marketplace/update_preflight_test.go"} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-03-run-stop-windows-leaves-test-endpoint.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-03-run-stop-windows-leaves-test-endpoint.json new file mode 100644 index 0000000000..e5bbc2bf65 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-03-run-stop-windows-leaves-test-endpoint.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-11-03", "symptom": "ako/mxcli#1917 (mendixlabs/mxcli#1723): after `run --local --test-endpoint --detach` and `run stop` on Windows the project kept module MxTest, AfterStartupMicroflow 'MxTest.RegisterEndpoint', themesource/mxtest/ and javasource/mxtest/ — git status dirty, and once MxTest was dropped the next build failed CE1613 on the after-startup. Reported success throughout.", "cause": "The endpoint is removed only by HostedEndpoint.Remove inside the dev-loop process (a defer and the os.Exit path). On Windows terminateProcess is `taskkill /T /F` — the code comment even said the teardown does not run — so a detached run never got to Remove; on any platform a SIGKILL after stopGrace or a crash skipped it too. Nothing outside the process knew what to undo.", "file": "cmd/mxcli/testrunner/host_restore.go (restore record + RestoreAbandonedEndpoint), cmd/mxcli/testrunner/host.go (record written before install, removed by Remove, stale one cleaned before a new install), cmd/mxcli/cmd_run_lifecycle.go (runStop restores after the process is gone)", "insight": "Teardown that lives in a process's exit path needs a durable record the moment it has something to undo, because a stop the process cannot see (taskkill /F, SIGKILL, crash) is a normal stop on one platform. The record carries the pre-install .mpr bytes and tree digest so the cross-process restore is as byte-exact as the in-process one, and it is gated on the owner PID being dead so a live run's endpoint is never pulled. The Windows bug reproduces on Linux by SIGKILL-ing the detached run before `run stop` — that is what taskkill /F is — so the integration test runs where mxbuild does; the graceful SIGTERM stop is the control (it already restored). Revert proof: a binary with only the runStop call stubbed fails the killed-run phase with the reported leftovers; beware a stub that does not compile (unused import) — the test then fails in 9 s for the wrong reason. The second half of the issue (dist/pages 404 after `test --attach`) is a separate cause: runAttached's applyModelChange calls serve.Build directly, bypassing the bundler/serve-recovery path the run's own reload uses; reproduced on Linux 11.12.6.", "refs": ["ako/mxcli#1917", "mendixlabs/mxcli#1723", "mendixlabs/mxcli#1645"], "test": "cmd/mxcli/cmd_run_test_endpoint_integration_test.go (integration && linux, mxbuild 11.12.6); cmd/mxcli/testrunner/host_restore_test.go"} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-03-test-attach-404-page-bundles.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-03-test-attach-404-page-bundles.json new file mode 100644 index 0000000000..f646af0db7 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-03-test-attach-404-page-bundles.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-11-03", "symptom": "ako/mxcli#1917 (mendixlabs/mxcli#1723): after a green `mxcli test --attach` against `run --local --test-endpoint`, the hosting app answered 404 on dist/pages/..js and every page showed the generic error dialog until the app was restarted. Reproduced on Linux with Mendix 11.12.6 (reported on Windows): dist/pages held 0 page modules after the attach.", "cause": "runAttached's applyModelChange built with a bare ServeServer pointed at the dev loop's serve port. Every attach build is a later build in that mxbuild --serve process, and on the prebuilt-client versions a later build replaces web/dist with a monolithic bundle without pages (mendixlabs/mxcli#1580). The dev loop's own reloads go through serveRecovery, which restarts the serve process on that outcome; the attach bypassed it, and could not reuse it as is, because the serve process belongs to the other mxcli.", "file": "cmd/mxcli/docker/serve_attached.go (AttachedServe: serveRecovery over a borrowedServe whose Restart starts a private mxbuild --serve on a free port instead of touching the host's; Close stops only that), cmd/mxcli/testrunner/runner_attach.go (builds through it, closes it when the attach ends)", "insight": "A recovery that lives in one process's loop does not travel with the API it drives: the attach called the same /build endpoint and inherited the defect but not the fix. Reuse the recovery and swap only its Restart for one this process owns — the host's serve is left in its stale state, which its own serveRecovery already handles on its next build. The cleanup build lands on the private serve as a later build too, so it restarts that one: two cold builds per attach, the price #1580 already set (correct beats fast). Integration test drives the reporter's sequence with a real 11.12.6 runtime and GETs a page module the boot produced (control: 200 before the attach); a binary whose AttachedServe.Build bypasses only the recovery fails it with 0 page modules left. A lingering java process after the run is mxbuild's Gradle daemon, not a leaked serve.", "refs": ["ako/mxcli#1917", "mendixlabs/mxcli#1723", "mendixlabs/mxcli#1580"], "test": "cmd/mxcli/cmd_test_attach_bundle_integration_test.go (integration && linux, mxbuild 11.12.6); cmd/mxcli/docker/serve_attached_test.go"} diff --git a/.claude/skills/fix-issue/findings/mdl-backend/2026-11-03-module-settings-jar-write-wipes-packageid-checksum.json b/.claude/skills/fix-issue/findings/mdl-backend/2026-11-03-module-settings-jar-write-wipes-packageid-checksum.json new file mode 100644 index 0000000000..bc2702e137 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-backend/2026-11-03-module-settings-jar-write-wipes-packageid-checksum.json @@ -0,0 +1 @@ +{"area": "mdl/backend", "date": "2026-11-03", "refs": ["ako/mxcli#1920"], "ce": [], "symptom": "`alter module … add | set | drop jar dependency` reports success and mx check stays clean, but the module's Projects$ModuleSettings loses PackageId (Studio Pro's Module ID) and, on a Marketplace module, its Checksum, and EnableDetailedTroubleshooting flips from false to true.", "file": "mdl/backend/modelsdk/module_settings_write.go (UpdateModuleSettings, moduleSettingsContents); test module_settings_carry_test.go with fixture testdata/TestApp.WorkflowCommons.ModuleSettings.11_16.bson", "cause": "UpdateModuleSettings marshalled a fresh bson.M of the nine keys types.ModuleSettings models; the six it does not (PackageId, Checksum, ConvertedChecksum, OriginalPackageId, ModuleDependencies, EnableDetailedTroubleshooting) were absent, and canon.CompletePropertySets put the names back with metamodel defaults rather than the stored values. The default enableDetailedTroubleshooting is true, which is why a false came back true.", "insight": "**A completed property set looks like a carry and is not one.** The rewritten unit had every key Studio Pro writes, so a key-presence check passed; only comparing VALUES against the stored unit showed the loss, and only on a module whose values differ from the defaults — an app module made by mxcli or Studio Pro holds defaults everywhere but PackageId, so the reproduction has to be a Marketplace module (WorkflowCommons in ako/TestApp: a Checksum, a PackageId and troubleshooting off). The fix starts the document from the stored keys and overwrites only the modelled ones, rather than adding six more fields to the rebuild, so the next key Studio Pro adds is carried too. One writer only (cmd_modules.go is its sole caller); CreateModule still writes a new module without a PackageId, unmeasured against Studio Pro. Control: the extracted function ignoring the stored bytes fails the fixture test on Checksum, PackageId, EnableDetailedTroubleshooting, ConvertedChecksum and OriginalPackageId; end to end on a TestApp copy the jar statement now changes only JarDependencies and mx check (11.16.0 Beta) reports 0 errors."} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-03-call-external-action-include-associations-reset.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-03-call-external-action-include-associations-reset.json new file mode 100644 index 0000000000..5f3f92533e --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-03-call-external-action-include-associations-reset.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-11-03", "refs": ["ako/mxcli#1914", "mendixlabs/mxcli#1721"], "ce": [], "symptom": "A `call external action` whose object parameter has \"Associations to include\" ticked in Studio Pro loses them on every `create or modify microflow` that restates the call: the OData request then carries only the object's own attributes and silently drops the associated objects at runtime. DESCRIBE shows nothing, and check, lint and docker check all pass.", "file": "mdl/executor/external_action_carry.go (carryIncludedAssociations, wired in cmd_microflows_build.go); mdl/backend/modelsdk/microflow_read_actions.go (includedAssociationsFromGen); mdl/backend/modelsdk/microflow_external_action_write.go (includedAssociationsToGen, marker-2 lists); sdk/microflows ExternalActionParameterMapping.IncludedAssociations", "cause": "The semantic model had no field for Microflows$ExternalActionParameterMapping.IncludedAssociations, the reader did not read it, callExternalActionToGen built each mapping from scratch, and the rebuild had no MDL to take it from. Same class as rewrite-drops-unauthored-state.", "insight": "**The report called it a boolean flag; it is a tree.** The Studio Pro reference (ako/TestApp Odata.IncludeAssociationsMF, 11.16.0 Beta) stores a list of Microflows$IncludedAssociation {Association, IncludedAssociations, IsParent} per parameter mapping, an association ticked under another nested in its IncludedAssociations; marker 2 at both levels; IsParent true where the object on that side is the association's FROM (ParentPointer) entity. The control microflow (_Off, nothing ticked) differs only in that list, so one property is the whole test. Building the reference took two contracts: Studio Pro refuses a ComplexType action parameter (R017 \"One of the parameters is not supported\"), so the reporter's DTOs are entity types without an entity set. **Matching stored to rebuilt calls cannot use position**: a flow's stored object order is Studio Pro's, not the statement's. The carry keys on service.action and parameter name; when two stored calls to one action hold different trees it refuses, and a shared tree carried twice gets fresh $IDs on the second copy. A tree is not carried onto a parameter whose resolved entity changed. **Under `mdl 1;` the rewrite usually does not reach the rebuild at all**: create or modify splices into the stored flow by re-parsing DESCRIBE's output, and for this reference that output is `additional = )` (Studio Pro 11.16 stored an unfilled nullable argument as \"\", which DESCRIBE prints bare), so every mdl-1 rewrite of the microflow is refused before anything is lost; the carry covers headerless scripts and the rebuild fallback. ALTER passes the untouched call through as stored bytes and was never affected. **mx check could not be the end-to-end check**: Studio Pro 11.16.0 Beta's mx crashes (NullReferenceException in DocumentCheck) on the untouched TestApp too. Controls: disabling the carry call fails TestCreateOrModify_CarriesIncludedAssociations; disabling the writer's list fails TestCallExternalAction_IncludedAssociationsRoundTrip on the On fixture while the Off fixture passes. **Left out on purpose**: AdditionalAttributes (no reference holds one), the action-level IncludedAssociations (empty in both references), and the empty marker lists Studio Pro writes on every mapping (version-gated: 10.11+ / 11.5+, unwritten since #1073)."} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-03-folded-decision-condition-edit-inverts.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-03-folded-decision-condition-edit-inverts.json new file mode 100644 index 0000000000..56896e6a72 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-03-folded-decision-condition-edit-inverts.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-11-03", "symptom": "ako/mxcli#1930 (mendixlabs/mxcli#1728): a validation decision (`if trim($X/Code) != '' then else set $Valid = false; end if;`) is described folded, `if not(trim($X/Code) != '') then set $Valid = false; end if;`. Feeding that description back through `create or modify microflow` with only the condition text changed stored `not(trim($X/Reference) != '')` as the decision's expression while its true/false flows stayed: the decision was inverted, and check --references, exec (\"spliced: 1 edited in place\") and diff all looked harmless.", "cause": "describe's empty-then swap (true path straight to the merge, false path with content) prints negateIfCondition(expr) and swaps the paths, but the splice's in-place condition edit (patchDiff.conditionEdit → splitCondition → SetCondition, ako/mxcli#888) stored the script's condition text as it was. The flows are untouched by a condition edit, so the printed negation went into the model un-negated.", "file": "mdl/executor/cmd_alter_flow.go (splitCondition negates when decisionPrintedNegated), mdl/executor/cmd_microflows_show_helpers.go (negateExpression extracted from negateIfCondition; decisionPrintedNegated recomputes describe's fold from the stored flows with findSplitMergePoints + findBranchFlows)", "insight": "A describer that rewrites what it prints (here: negate and swap for readability) creates an obligation on every path that reads its output back: the inverse transform. Decide the fold from the stored flows with describe's own merge computation, never from the text — describe also strips a stored not(Z) to Z, so 'the printed condition starts with not(' detects neither case reliably. negateExpression is its own inverse, which is what makes the round trip exact: printed not(Y) stores Y, printed Z stores not(Z). Test: create the reporter's flow on a pedapp copy, describe, change only the attribute in the printed condition, exec under mdl 1; assert the decision kept its $ID and stores the un-negated expression, for both fold shapes; with the negation stubbed both fail with the inversion. A decision in a loop body is not edited at all (the splice refuses), pinned by a second test.", "refs": ["ako/mxcli#1930", "mendixlabs/mxcli#1728", "ako/mxcli#888", "ako/mxcli#1761"], "test": "mdl/executor/flow_splice_negated_condition_pedapp_test.go"} diff --git a/.claude/skills/fix-issue/findings/mdl-grammar/2026-11-03-member-named-error-lexes-as-error-message-clause.json b/.claude/skills/fix-issue/findings/mdl-grammar/2026-11-03-member-named-error-lexes-as-error-message-clause.json new file mode 100644 index 0000000000..586b1a9e55 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-grammar/2026-11-03-member-named-error-lexes-as-error-message-clause.json @@ -0,0 +1 @@ +{"area": "mdl/grammar", "date": "2026-11-03", "symptom": "ako/mxcli#1913 (mendixlabs/mxcli#1720): `validation feedback $Entity2/Error message 'not good';` failed check with \"missing MESSAGE at ''not good''\" for an attribute named Error (any case) — exactly what describe writes for such an attribute, so describe output for the microflow did not re-parse; quoting it (`$Entity2/\"Error\"`) worked.", "cause": "ERROR_MESSAGE is one compound lexer token (`E R R O R WS+ M E S S A G E`, for the `error message` clause). The lexer has no context, so the member name and the MESSAGE keyword after it were swallowed into it and the path ended at `$Entity2/`.", "file": "mdl/grammar/MDLLexer.g4 (right-edge predicate {!p.followsMemberSeparator()}? on the grouped ERROR_MESSAGE alternatives, helper in @lexer::members reading TokenStartCharIndex); cmd/gen-completions/main.go (firstAlternative strips the group's opening paren, else ERROR MESSAGE dropped out of lsp_completions_gen.go)", "insight": "A multi-word lexer token is a keyword collision that no keyword list shows: `Error` and `message` are each fine as names, only the pair collides, so lexesAsBareIdentifier/mdlIdent cannot see it and quoting in describe would not have helped an unquoted script. Fixing it in the lexer, not the one parser rule, covers every position an attribute path can take. Two traps in the predicate: skipping line breaks backwards lets a line comment ending in '.' or '/' (`-- see the docs.`) read as a separator and break the clause on the next line, and a block comment's closing `*/` ends in a slash — both are controls in the test, and the newline-skipping draft failed them. `$` inside an @lexer::members comment is an ANTLR attribute reference (error 128), and gen-completions reconstructs a multi-line rule from its first line, so a grouped `( E R R O R …` must have its paren stripped there. **A lexer predicate goes on the right edge**: the first version put it on the left edge, which ANTLR evaluates at every token start, disabling the lexer DFA cache — `check` over 300 example scripts went from 43 s to 91 s and the conformance and upgrade corpus tests hit the 30-minute timeout, while every unit test passed. On the right edge it runs only after `error message` matched (42 s vs 44 s, noise); the token start then comes from TokenStartCharIndex, since LA(j<0) is relative to the current index. Time a lexer change against a control binary, not just its tests. Proven by stubbing the predicate to false (the reported 'missing MESSAGE' returns in the visitor and describe round-trip tests) and by a control-binary sweep of all 1210 .mdl files in the repo: identical results.", "refs": ["ako/mxcli#1913", "mendixlabs/mxcli#1720", "docs-wiki/bug-patterns/keyword-collisions.md"], "test": "mdl/visitor/error_member_name_test.go; mdl/executor/cmd_microflows_format_action_test.go (TestFormatAction_ValidationFeedback_MemberNamedErrorReparses); cmd/gen-completions/predicate_test.go"} diff --git a/.claude/skills/mendix/run-local/SKILL.md b/.claude/skills/mendix/run-local/SKILL.md index d2b5c66760..99baffe0f0 100644 --- a/.claude/skills/mendix/run-local/SKILL.md +++ b/.claude/skills/mendix/run-local/SKILL.md @@ -12,27 +12,27 @@ description: "The warm Docker-free dev loop, `mxcli run --local` — model chang |---|---| | 40-47 | Overview | | 48-60 | When to Use This Skill | -| 61-126 | Usage | -| 127-139 | How apply is chosen | -| 140-230 | Prerequisites | -| 231-248 | The intended loop | -| 249-276 | Editing themes (SCSS) — rebuild, never clear caches | -| 277-303 | `--watch` waits for a write to finish before it builds | -| 304-332 | "My edit didn't show up" — stale process, not stale cache | -| 333-363 | Flags | -| 364-417 | Metrics and OpenTelemetry | -| 418-475 | Pages render in the browser | -| 476-493 | Pixel-perfect page loop | -| 494-514 | Debugging a server-side error | -| 515-538 | "Sign in failed" that is not about the password | -| 539-571 | The same licence also stops the app after a few hours | -| 572-651 | External browser preview (`--hub`) | -| 652-660 | Validation checklist | -| 661-717 | Constant values come from a configuration | -| 718-721 | Warm local dev loop (`mxcli run --local [--watch] [--screenshot]`) | -| 722-725 | External browser preview (`mxcli run --hub ` + `mxcli tunnel-hub`) | -| 726-729 | Tunnel-hub GitHub authentication (opt-in, gated on `--github-oauth-client-id`; absent = today's open hub) | -| 730-732 | Runtime metrics + settings passthrough (`mxcli run --local --metrics` / `--runtime-setting Key=Value`) | +| 61-127 | Usage | +| 128-140 | How apply is chosen | +| 141-231 | Prerequisites | +| 232-249 | The intended loop | +| 250-277 | Editing themes (SCSS) — rebuild, never clear caches | +| 278-304 | `--watch` waits for a write to finish before it builds | +| 305-333 | "My edit didn't show up" — stale process, not stale cache | +| 334-364 | Flags | +| 365-418 | Metrics and OpenTelemetry | +| 419-476 | Pages render in the browser | +| 477-494 | Pixel-perfect page loop | +| 495-515 | Debugging a server-side error | +| 516-539 | "Sign in failed" that is not about the password | +| 540-572 | The same licence also stops the app after a few hours | +| 573-652 | External browser preview (`--hub`) | +| 653-661 | Validation checklist | +| 662-718 | Constant values come from a configuration | +| 719-722 | Warm local dev loop (`mxcli run --local [--watch] [--screenshot]`) | +| 723-726 | External browser preview (`mxcli run --hub ` + `mxcli tunnel-hub`) | +| 727-730 | Tunnel-hub GitHub authentication (opt-in, gated on `--github-oauth-client-id`; absent = today's open hub) | +| 731-733 | Runtime metrics + settings passthrough (`mxcli run --local --metrics` / `--runtime-setting Key=Value`) | # Warm Local Dev Loop — `mxcli run --local` @@ -112,7 +112,8 @@ What each one does, precisely: - **`run stop`** asks the run to shut down (it removes `--test-endpoint`, turns the debugger off), waits until it is gone, then sweeps the run's process session, so an orphaned `runtimelauncher` or mxbuild JVM is stopped too — including the leftovers of - a run that was killed with `-9`. Stopping when nothing runs is a no-op (exit 0). + a run that was killed with `-9` (and its `--test-endpoint`, as on Windows, where the + stop is always forced). Stopping when nothing runs is a no-op (exit 0). - **`run restart`** reuses the arguments recorded in the state file; to change a flag, `run stop` and `--detach` again. diff --git a/.claude/skills/mendix/write-microflows/reference/pitfalls.md b/.claude/skills/mendix/write-microflows/reference/pitfalls.md index 6924337a05..df9b0e6732 100644 --- a/.claude/skills/mendix/write-microflows/reference/pitfalls.md +++ b/.claude/skills/mendix/write-microflows/reference/pitfalls.md @@ -881,8 +881,9 @@ end; removes both. - **`export level api | hidden`** decides whether the microflow is part of the module's public surface when the module is exported. It bites in a - **Protected (add-on) module** — `describe module` prints - `-- export level: Protected …` and `show modules` says `Protected AddOn`: + **Protected (add-on) module** — `describe module` prints `ModuleType: addon` + (set it with `alter module M set (ModuleType: addon)`) and `show modules` + says `Protected AddOn`: another module or the navigation referencing a Hidden microflow, entity, attribute, association or any page there is **CE1406**, and an `api` microflow whose parameter or return type is a Hidden entity or enumeration of diff --git a/.claude/skills/steward/SKILL.md b/.claude/skills/steward/SKILL.md index 3f6f1f110f..d5176fff89 100644 --- a/.claude/skills/steward/SKILL.md +++ b/.claude/skills/steward/SKILL.md @@ -12,9 +12,9 @@ description: Drive an mxcli pull request Claude opened to green and merge it. Co |---|---| | 27-55 | Merge on green is the default | | 56-73 | Before every push: `make ci-local` | -| 74-88 | Reading a red CI job | -| 89-104 | Conflicts this repo keeps hitting | -| 105-122 | The AI PR Review comment | +| 74-89 | Reading a red CI job | +| 90-105 | Conflicts this repo keeps hitting | +| 106-123 | The AI PR Review comment | # Steward: Driving a PR to Merged @@ -78,12 +78,13 @@ 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). +- **`tunnel-seam-cross-platform (windows-latest | macos-latest)`:** runs the + tunnel seam's own tests on that platform (ADR-0009), so a test failure is + real. A PR runs only the Windows leg; macOS runs on `main` and nightly + (`tunnel-seam-macos`), because its hosted queue is the one known runner + flake: it sometimes cancels the job before it gets a runner (runner id 0, no + step ran). On `main` that is a job that died before any test body ran: + re-run the failed jobs **once**. - **`AI PR Review`** is always green: it posts a comment, it does not gate. ## Conflicts this repo keeps hitting diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 6ea88b119b..44a08a32c7 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -128,6 +128,22 @@ jobs: run: make test-integration timeout-minutes: 60 + # The tunnel seam's !linux stub on macOS (ADR-0009). Per-PR CI runs it on + # Windows only; push-test.yml runs macOS on main, and this catches a + # darwin-only break within a day even when main is quiet. Like + # upgrade-full-corpus it does not gate the release: the hosted macOS queue is + # the one known runner flake, and a nightly is not held back by it. + tunnel-seam-macos: + runs-on: macos-latest + timeout-minutes: 30 + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-go@v7 + with: + go-version: '1.27.3' + - name: Test the tunnel seam + run: bash scripts/test-tunnel-seam.sh + # The upgrade property test over the WHOLE mdl-examples corpus, header-only # scripts included (MXCLI_UPGRADE_ALL). Per-PR CI executes only the scripts # the upgrade rewrites beyond the header and terminators; this is the run that diff --git a/.github/workflows/push-test.yml b/.github/workflows/push-test.yml index b563b4839c..b9751f69e6 100644 --- a/.github/workflows/push-test.yml +++ b/.github/workflows/push-test.yml @@ -86,7 +86,13 @@ jobs: strategy: fail-fast: false matrix: - os: [windows-latest, macos-latest] + # macOS runs on main and workflow_dispatch only, and nightly + # (nightly.yml): on a PR, Windows already executes the same !linux stub, + # so macOS added only a darwin compile, while its hosted queue is the + # scarcest (5 concurrent on GitHub Free) and the one known flake, a job + # cancelled before it gets a runner. A darwin-only break is still caught + # on the merge to main. + os: ${{ fromJSON(github.event_name == 'pull_request' && '["windows-latest"]' || '["windows-latest", "macos-latest"]') }} runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v7 @@ -95,43 +101,7 @@ jobs: go-version: '1.27.3' - name: Test the tunnel seam shell: bash - # Scoped with -run to the seam's own tests. The full test binaries are still - # COMPILED for this platform, so a Windows/macOS build break is still caught; - # only the !linux stub behaviour is executed. - # - # Running the whole packages here fails on Windows for reasons that predate - # this change and are unrelated to the tunnel: several tests assert POSIX file - # modes (0600) that Windows does not implement — os.Chmod only toggles the - # read-only bit, so Stat reports 666 — plus one path-separator assumption. - # Tracked separately in #897; widening this job is that issue's job, not this - # one's. - # - # -run can pass vacuously if the tests are renamed or deleted, so assert that - # the expected number actually ran. - # - # `tee` rather than a command substitution: the runner's shell is - # `bash -e -o pipefail`, so capturing into `out=$(go test ...)` aborts the - # step AT THE ASSIGNMENT when go test fails, and the `echo "$out"` below - # never runs. The log then holds nothing but "Process completed with exit - # code 1" — no test name, no failure message, no way to tell a real break - # from a flake. Write the output as it is produced, then judge it. - run: | - set +e - go test -v -count=1 -run 'Unsupported' ./cmd/mxcli/docker/... ./cmd/mxcli/tunnelhub/... 2>&1 | tee go-test-output.txt - status=${PIPESTATUS[0]} - set -e - n=$(grep -c '^--- PASS: Test.*Unsupported' go-test-output.txt || true) - echo "seam tests executed: $n" - if [ "$status" -ne 0 ]; then - echo "FAIL: go test exited $status. Its output is above." - exit "$status" - fi - if [ "$n" -lt 4 ]; then - echo "FAIL: expected at least 4 tunnel-seam tests to run, -run matched $n." - echo " The !linux stubs in cmd/mxcli/docker and cmd/mxcli/tunnelhub" - echo " must each keep a test whose name contains 'Unsupported'." - exit 1 - fi + run: bash scripts/test-tunnel-seam.sh # The Windows process helpers (cmd/mxcli/docker/procgroup_windows.go) cannot be # exercised by the ubuntu job: Signal(0) succeeds on Linux and there is no diff --git a/CHANGELOG.md b/CHANGELOG.md index 43c7d59eeb..13323aa8d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,19 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Added + +- **A module's version and module type from MDL** (ako/mxcli#1921) — the General tab of Studio Pro's *Module settings* dialog: `create or modify module Inventory (Version: '2.3.1', ModuleType: addon)` and `alter module Inventory set (Version: '2.4.0')`. `describe module` prints the settings that differ from a new module's (App, 1.0.0), and the location as a comment when it is *Marketplace modules*; `list modules` gains `Version` and `Type` columns. The version must be SemVer, the type `app` or `addon` (Mendix 9.12+), and a statement that restates the stored settings writes nothing. Measured on 11.16: the type is the settings' `ExportLevel` (`Source` / `Protected`), not `ProtectedModuleType`, which is `AddOn` on every module. + ### Fixed +- **`marketplace update` no longer removes the module and then fails** (ako/mxcli#1934, mendixlabs/mxcli#1732) — every update failed with `open package target.mpk: no such file or directory` after it had already dropped the installed module, leaving the project half-updated: it looked for the downloaded package under a different name than it was saved under. It now uses the downloaded package, and checks the package and the new version's module before it changes anything, so an update that cannot complete stops with the project untouched and says so. +- **`create or modify microflow` no longer inverts a decision whose condition changes** (ako/mxcli#1930, mendixlabs/mxcli#1728) — `describe` prints a decision whose true path goes straight on as `if not(X) then …` with its paths swapped. Fed back with only the condition changed, the in-place edit stored the printed `not(Y)` as the decision's expression while its paths stayed, so the decision did the opposite of what the script said, with `check`, `exec` and `diff` all clean. The edit now stores the condition the decision means. +- **`test --attach` no longer breaks the hosting app's pages** (ako/mxcli#1917, mendixlabs/mxcli#1723) — after a green `mxcli test --attach` against `run --local --test-endpoint`, the app answered 404 on its page bundles and every page showed the generic error dialog until it was restarted. The attach's builds went to the app's own `mxbuild --serve`, where on Mendix 11.12.5 and later any build after the first leaves the client bundle without its pages. The attach now finishes such a build on a `mxbuild --serve` of its own, which it stops afterwards, and leaves the app's untouched; this costs a cold build each time it happens. +- **`run stop` removes `--test-endpoint` from the project on Windows** (ako/mxcli#1917, mendixlabs/mxcli#1723) — a detached `run --local --test-endpoint` stopped with `run stop` left the `MxTest` module, its after-startup microflow, `themesource/mxtest/` and `javasource/mxtest/` in the project, because on Windows the stop ends the run without its teardown; the next build then failed with CE1613 once the module was dropped. The run now records how to remove the endpoint when it installs it, and `run stop` (or the next `--test-endpoint` run) removes it when the run could not, on any platform, leaving the project file byte-identical. +- **A jar dependency change keeps the module's Module ID and Marketplace checksum** (ako/mxcli#1920) — `alter module … add | set | drop jar dependency` rebuilt the module's settings from the keys mxcli models, so the rest came back as defaults: the Module ID (`PackageId`) and a Marketplace module's `Checksum` were wiped, and detailed troubleshooting was switched on. The change is now written onto the stored settings, and only the jar dependencies change. +- **`create or modify microflow` keeps a call external action's "Associations to include"** (ako/mxcli#1914, mendixlabs/mxcli#1721) — MDL has no syntax for them, so a rewrite that restated the call wrote it with none, and the OData request then silently left out the associated objects; `check`, `lint` and the build all passed. The associations Studio Pro stored on each parameter — a tree, not a flag: an association ticked under another is nested in it — are now carried onto the rewritten call, matched by action and parameter name, and only while the parameter still binds the same entity. When a microflow calls the same action twice with different associations, the rewrite is refused rather than guessing which call has which. A drop followed by a create still starts without them. +- **`validation feedback` on an attribute named `Error` parses** (ako/mxcli#1913, mendixlabs/mxcli#1720) — `validation feedback $E/Error message 'x'`, which is what `describe` writes for such an attribute, failed with "missing MESSAGE" because `Error message` was read as the `error message` clause keyword. After a `/` or `.` the word is now always a member name, in any case and anywhere an attribute path appears; the `error message` clause itself is unchanged. - **`dateFormat: custom` on a dynamic-text parameter is stored as Custom** (ako/mxcli#1864, mendixlabs/mxcli#1706) — `check` accepts a parameter's `dateFormat` and `enumFormat` in any case, but the writer stored the value as written, and `"custom"` is not a value Mendix knows, so the runtime showed the default format. The values are now stored in Mendix's spelling (`Custom`, `DateTime`, `Image`) on dynamic text and on Data grid 2 column templates. - **A doc comment above `alter entity … add attribute` is stored on the attribute** (ako/mxcli#1863, mendixlabs/mxcli#1705) — `/** Why */` written above the statement was read by nothing: exec reported "Added attribute" and the attribute had no documentation, with only a later `check` warning MDL089. It now documents the attribute the statement adds, as it does above a `create entity` attribute, for `alter entities` too. When the statement adds two attributes, or the attribute has its own comment, the comment is still not stored and `check` still warns. diff --git a/cmd/gen-completions/main.go b/cmd/gen-completions/main.go index 0b5e6c137f..0dffc853d7 100644 --- a/cmd/gen-completions/main.go +++ b/cmd/gen-completions/main.go @@ -207,8 +207,7 @@ func parseLexerGrammar(path string) ([]tokenEntry, error) { } // Try to reconstruct from the first alternative - firstAlt := strings.TrimSuffix(strings.TrimSpace(body), "|") - firstAlt = strings.TrimSpace(firstAlt) + firstAlt := firstAlternative(body) text := reconstructKeyword(tokenName, firstAlt) if text != "" { entries = append(entries, tokenEntry{ @@ -223,6 +222,22 @@ func parseLexerGrammar(path string) ([]tokenEntry, error) { return entries, scanner.Err() } +// groupOpenRE matches the paren that opens a grouped rule body, as in +// `ERROR_MESSAGE: ( E R R O R WS+ M E S S A G E` whose group carries a +// right-edge predicate. +var groupOpenRE = regexp.MustCompile(`^\(\s+`) + +// firstAlternative returns the keyword spelling on the first line of a +// multi-line lexer rule: the body with a trailing `|` and an opening group +// paren removed. Without the paren strip a predicated keyword silently dropped +// out of the LSP completions (mendixlabs/mxcli#1720). +func firstAlternative(body string) string { + alt := strings.TrimSpace(body) + alt = groupOpenRE.ReplaceAllString(alt, "") + alt = strings.TrimSuffix(alt, "|") + return strings.TrimSpace(alt) +} + // reconstructKeyword converts a token body (the RHS of the lexer rule) to user-facing text. // // Examples: diff --git a/cmd/gen-completions/predicate_test.go b/cmd/gen-completions/predicate_test.go new file mode 100644 index 0000000000..79f2ff4630 --- /dev/null +++ b/cmd/gen-completions/predicate_test.go @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "os" + "path/filepath" + "testing" +) + +// A grouped keyword rule guarded by a predicate must still yield its +// completion: ERROR_MESSAGE gained one for mendixlabs/mxcli#1720 and the +// generator, reading `( E R R O R …` as an unknown part, dropped +// "ERROR MESSAGE" from the LSP keyword list. +func TestPredicatedMultiLineRuleKeepsItsKeyword(t *testing.T) { + src := "lexer grammar L;\n" + + "ERROR_MESSAGE: ( E R R O R WS+ M E S S A G E\n" + + " | E R R O R '_' M E S S A G E\n" + + " ) {!p.followsMemberSeparator()}?;\n" + + "CASCADE: C A S C A D E;\n" + path := filepath.Join(t.TempDir(), "L.g4") + if err := os.WriteFile(path, []byte(src), 0o644); err != nil { + t.Fatal(err) + } + entries, err := parseLexerGrammar(path) + if err != nil { + t.Fatal(err) + } + got := map[string]string{} + for _, e := range entries { + got[e.Name] = e.Text + } + if got["ERROR_MESSAGE"] != "ERROR MESSAGE" { + t.Errorf("ERROR_MESSAGE completion = %q, want %q (entries: %v)", got["ERROR_MESSAGE"], "ERROR MESSAGE", got) + } + if got["CASCADE"] != "CASCADE" { + t.Errorf("control: CASCADE = %q", got["CASCADE"]) + } +} diff --git a/cmd/mxcli/cmd_marketplace_diff.go b/cmd/mxcli/cmd_marketplace_diff.go index 33ed2fcb5d..6cc60b4e21 100644 --- a/cmd/mxcli/cmd_marketplace_diff.go +++ b/cmd/mxcli/cmd_marketplace_diff.go @@ -107,7 +107,7 @@ func runMarketplaceDiff(cmd *cobra.Command, args []string) error { if err != nil { return err } - baseRef, basePkgModule, err := referenceFor(ctx, client, base, mendixVersion, work, "b") + baseRef, basePkgModule, _, err := referenceFor(ctx, client, base, mendixVersion, work, "b") if err != nil { return err } @@ -132,7 +132,7 @@ func runMarketplaceDiff(cmd *cobra.Command, args []string) error { if err != nil { return err } - targetRef, targetPkgModule, err := referenceFor(ctx, client, targetVersion, mendixVersion, work, "t") + targetRef, targetPkgModule, _, err := referenceFor(ctx, client, targetVersion, mendixVersion, work, "t") if err != nil { return err } @@ -201,25 +201,28 @@ func downloadVersion(ctx context.Context, client *mp.Client, v *mp.Version, work // referenceFor downloads one version's package and builds a reference project // from it. Each version gets its own subdirectory so --to can hold two. It also // returns the module name the package declares, which is what the reference -// project's copy is called. +// project's copy is called, and where the package itself is: a caller that +// needs the package derives no path of its own, which is how `marketplace +// update` came to look for target.mpk while the package was t.mpk and fail +// after dropping the module (ako/mxcli#1934). func referenceFor(ctx context.Context, client *mp.Client, v *mp.Version, - mendixVersion, work, slot string) (mprPath, pkgModule string, err error) { + mendixVersion, work, slot string) (mprPath, pkgModule, mpkPath string, err error) { // The slot is one letter on purpose: mx writes the whole reference project // beneath refDir, and every character here is one less for the package's own // paths (see marketplace.NewWorkDir). refDir := filepath.Join(work, slot) if err := os.MkdirAll(refDir, 0o755); err != nil { - return "", "", err + return "", "", "", err } - mpkPath := filepath.Join(work, slot+".mpk") + mpkPath = filepath.Join(work, slot+".mpk") // A reference is expensive (a download plus two ~10s mx invocations) and // immutable, so the same published version is never built twice: `diff` // followed by `update` needs the same base, and so does re-running either. if cached := marketplace.CachedReference(v.VersionID, mendixVersion, refDir, mpkPath); cached != "" { if pkgModule, err = marketplace.ModuleNameInPackage(mpkPath); err == nil { - return cached, pkgModule, nil + return cached, pkgModule, mpkPath, nil } // The package came back unreadable, so the entry is not trustworthy. // Fall through and rebuild rather than failing the command. @@ -228,19 +231,19 @@ func referenceFor(ctx context.Context, client *mp.Client, v *mp.Version, } } - if _, err := downloadVersion(ctx, client, v, work, slot); err != nil { - return "", "", err + if mpkPath, err = downloadVersion(ctx, client, v, work, slot); err != nil { + return "", "", "", err } pkgModule, err = marketplace.ModuleNameInPackage(mpkPath) if err != nil { - return "", "", err + return "", "", "", err } mprPath, err = marketplace.PackageProject(ctx, mpkPath, mendixVersion, refDir, newBackendFactory()) if err != nil { - return "", "", err + return "", "", "", err } marketplace.CacheReference(v.VersionID, mendixVersion, refDir, mpkPath) - return mprPath, pkgModule, nil + return mprPath, pkgModule, mpkPath, nil } // versionIDs projects the marketplace's version list to bare version UUIDs. diff --git a/cmd/mxcli/cmd_marketplace_update.go b/cmd/mxcli/cmd_marketplace_update.go index 7ebf518bdd..8b3d678565 100644 --- a/cmd/mxcli/cmd_marketplace_update.go +++ b/cmd/mxcli/cmd_marketplace_update.go @@ -3,10 +3,10 @@ package main import ( + "errors" "fmt" "io" "os" - "path/filepath" "strings" "github.com/mendixlabs/mxcli/cmd/mxcli/marketplace" @@ -135,7 +135,7 @@ func runMarketplaceUpdate(cmd *cobra.Command, args []string) error { " downloadable. It is not, and --force does not help: there is nothing to compare against.\n"+ " hint: re-run with --no-baseline to update without that check (local edits are lost silently)", berr) } - baseRef, basePkgModule, rerr := referenceFor(ctx, client, base, mendixVersion, work, "b") + baseRef, basePkgModule, _, rerr := referenceFor(ctx, client, base, mendixVersion, work, "b") if rerr != nil { return rerr } @@ -167,19 +167,23 @@ func runMarketplaceUpdate(cmd *cobra.Command, args []string) error { if err != nil { return err } - targetRef, _, err := referenceFor(ctx, client, targetVersion, mendixVersion, work, "t") + // The bundled files and widgets are taken from the package rather than from + // the reference project, whose widgets/ also holds the blank template's. Its + // path is the one referenceFor downloaded to: this used to be spelled out + // here as target.mpk while the download was t.mpk, so every update dropped + // the module and then failed to open the package (ako/mxcli#1934). + targetRef, _, targetMpk, err := referenceFor(ctx, client, targetVersion, mendixVersion, work, "t") if err != nil { return err } - // referenceFor downloaded the package into the work directory under this - // slot; the widgets are taken from it rather than from the reference project, - // whose widgets/ also holds the blank template's. - targetMpk := filepath.Join(work, "target.mpk") fmt.Fprintf(out, "\nUpdating %s %s → %s...\n", moduleName, installedVersion, target) res, err := marketplace.PerformUpdate(mprPath, targetRef, targetMpk, moduleName, installedVersion, target, targetVersion.VersionID, newBackendFactory()) if err != nil { + if errors.Is(err, marketplace.ErrUpdateNotStarted) { + return err + } return fmt.Errorf("%w\n\nThe project may be mid-update — %s could be missing. Restore from version control", err, moduleName) } diff --git a/cmd/mxcli/cmd_run_detach_windows.go b/cmd/mxcli/cmd_run_detach_windows.go index a46f6411e9..ec99a5b97c 100644 --- a/cmd/mxcli/cmd_run_detach_windows.go +++ b/cmd/mxcli/cmd_run_detach_windows.go @@ -33,8 +33,8 @@ func setDetachAttrs(cmd *exec.Cmd) { // terminateProcess ends the run and its whole process tree. Windows has no // SIGTERM, and the run's hidden console (CREATE_NO_WINDOW) is not one the // caller is attached to, so no Ctrl-Break can reach it either: this is not a -// graceful stop, and the run's own teardown (removing --test-endpoint) does -// not run. +// graceful stop, and the run's own teardown does not run: runStop removes +// --test-endpoint from the project afterwards instead (ako/mxcli#1917). func terminateProcess(pid int) error { return exec.Command("taskkill", "/T", "/F", "/PID", fmt.Sprint(pid)).Run() } diff --git a/cmd/mxcli/cmd_run_lifecycle.go b/cmd/mxcli/cmd_run_lifecycle.go index 66ffe4e236..3dce97f499 100644 --- a/cmd/mxcli/cmd_run_lifecycle.go +++ b/cmd/mxcli/cmd_run_lifecycle.go @@ -13,6 +13,7 @@ import ( "time" "github.com/mendixlabs/mxcli/cmd/mxcli/docker" + "github.com/mendixlabs/mxcli/cmd/mxcli/testrunner" "github.com/spf13/cobra" ) @@ -92,7 +93,9 @@ and the web client bundler. It asks the run to shut down (so it tears down its own children and removes anything it installed, like --test-endpoint), waits until they are gone, and for a --detach run then checks the run's whole process session, so no orphaned runtimelauncher or mxbuild JVM is left behind — even -from a run that was killed with -9 and never cleaned up. +from a run that was killed with -9 and never cleaned up. A --test-endpoint the +run could not remove itself (on Windows the stop is always a forced one) is +removed from the project here, from what the run recorded when it installed it. Stopping when nothing runs is a no-op that exits 0. @@ -663,6 +666,13 @@ func runStop(projectPath string, out io.Writer) int { fmt.Fprintf(out, "failed: %d process(es) of pid %d's run are still alive: %v\n", len(left), st.PID, left) return runExitFailed } + // A run stopped without its teardown — always on Windows, where the stop is + // taskkill /F, and after the SIGKILL fallback above — leaves --test-endpoint + // in the project; the run recorded how to take it out (ako/mxcli#1917). + if _, err := testrunner.RestoreAbandonedEndpoint(projectPath, out); err != nil { + fmt.Fprintf(out, "failed: %v\n", err) + return runExitFailed + } extra := "" if reaped > 0 { extra = fmt.Sprintf(", reaped %d leftover process(es)", reaped) diff --git a/cmd/mxcli/cmd_run_test_endpoint_integration_test.go b/cmd/mxcli/cmd_run_test_endpoint_integration_test.go new file mode 100644 index 0000000000..84a53d433d --- /dev/null +++ b/cmd/mxcli/cmd_run_test_endpoint_integration_test.go @@ -0,0 +1,184 @@ +//go:build integration && linux + +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "bytes" + "crypto/sha256" + "fmt" + "io/fs" + "os" + "os/exec" + "path/filepath" + "strings" + "syscall" + "testing" + "time" + + "github.com/mendixlabs/mxcli/cmd/mxcli/docker" +) + +// TestRunStop_RestoresTestEndpointOfAKilledRun: `run --test-endpoint` puts the +// MxTest module, its after-startup microflow and themesource/mxtest/ into the +// project and removes them on its own way out. On Windows `run stop` cannot ask +// for that way out: it ends the run with `taskkill /F`, the teardown never runs, +// and the project is left carrying the endpoint, which the next build reports as +// CE1613 once MxTest is dropped (ako/mxcli#1917, mendixlabs/mxcli#1723). A +// SIGKILL here is the same stop, so the test kills the run outright and asks +// `run stop` to leave the project as it was before the run. +// +// MXCLI_RUNLIFE_VERSION=11.12.6 go test -tags integration ./cmd/mxcli -run RestoresTestEndpoint -v +// +// The control is the plain `run stop` of a live run (SIGTERM), which already +// restored the project before this fix; the killed run did not. +func TestRunStop_RestoresTestEndpointOfAKilledRun(t *testing.T) { + version := os.Getenv("MXCLI_RUNLIFE_VERSION") + if version == "" { + version = "11.12.6" + } + mxPath, err := docker.ResolveMxForVersion("", version) + if err != nil { + t.Skipf("mxbuild %s is not cached: %v", version, err) + } + bin := os.Getenv("MXCLI_BIN") + if bin == "" { + bin = filepath.Join(t.TempDir(), "mxcli") + build := exec.Command("go", "build", "-o", bin, ".") + if out, err := build.CombinedOutput(); err != nil { + t.Fatalf("go build: %v\n%s", err, out) + } + } + work, err := os.MkdirTemp("", "mxte") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.RemoveAll(work) }) + create := exec.Command(mxPath, "create-project") + create.Dir = work + if out, err := create.CombinedOutput(); err != nil { + t.Skipf("mx create-project: %v\n%s", err, out) + } + mpr := filepath.Join(work, "App.mpr") + + const appPort, adminPort, servePort = 8387, 8397, 6843 + for _, p := range []int{appPort, adminPort, servePort} { + if portAnswers(p) { + t.Skipf("port %d is in use", p) + } + } + mxcli := func(args ...string) string { + cmd := exec.Command(bin, args...) + cmd.Dir = work + var out bytes.Buffer + cmd.Stdout, cmd.Stderr = &out, &out + if err := cmd.Run(); err != nil { + t.Fatalf("mxcli %v: %v\n%s", args, err, out.String()) + } + return out.String() + } + t.Cleanup(func() { _ = exec.Command(bin, "run", "stop", "-p", mpr).Run() }) + + before := projectFingerprint(t, work) + + mxcli("run", "--local", "--test-endpoint", "--detach", "--db-type", "hsqldb", + "--app-port", fmt.Sprint(appPort), "--admin-port", fmt.Sprint(adminPort), + "--serve-port", fmt.Sprint(servePort), "-p", mpr) + if !strings.Contains(mxcli("-p", mpr, "-c", "describe settings"), "MxTest.RegisterEndpoint") { + t.Fatal("precondition: the running app's project does not carry the test endpoint") + } + + st, err := docker.ReadRunState(mpr) + if err != nil || st == nil { + t.Fatalf("reading the run state: %v", err) + } + // What taskkill /F does on Windows: no signal the run can handle. + if err := syscall.Kill(st.PID, syscall.SIGKILL); err != nil { + t.Fatalf("killing the run: %v", err) + } + waitGone(func() bool { return docker.PidAlive(st.PID) }, 10*time.Second) + + out := mxcli("run", "stop", "-p", mpr) + + settings := mxcli("-p", mpr, "-c", "describe settings") + if strings.Contains(settings, "MxTest") { + t.Errorf("after-startup still points at the test endpoint after run stop:\n%s\nrun stop said:\n%s", settings, out) + } + if strings.Contains(mxcli("-p", mpr, "-c", "show modules"), "MxTest") { + t.Errorf("module MxTest is still in the project after run stop; run stop said:\n%s", out) + } + if after := projectFingerprint(t, work); after != before { + t.Errorf("the project differs from before the run:\n%s\nrun stop said:\n%s", diffFingerprints(before, after), out) + } + + // Control: a live run stopped normally (SIGTERM) removes its own endpoint, + // and with it the record, so nothing is left for a later stop to act on. + mxcli("run", "--local", "--test-endpoint", "--detach", "--db-type", "hsqldb", + "--app-port", fmt.Sprint(appPort), "--admin-port", fmt.Sprint(adminPort), + "--serve-port", fmt.Sprint(servePort), "-p", mpr) + out = mxcli("run", "stop", "-p", mpr) + if strings.Contains(out, "left in the project") { + t.Errorf("a graceful stop left its endpoint for run stop to remove:\n%s", out) + } + if _, err := os.Stat(filepath.Join(work, ".mxcli", "test-endpoint-restore.json")); !os.IsNotExist(err) { + t.Errorf("the restore record survives a graceful stop (stat: %v)", err) + } + if after := projectFingerprint(t, work); after != before { + t.Errorf("a graceful stop left the project changed:\n%s", diffFingerprints(before, after)) + } +} + +// projectFingerprint lists the files `run --test-endpoint` may touch — the +// .mpr, mprcontents/, themesource/mxtest and javasource/mxtest — with a hash of +// each, one per line. What mxbuild regenerates on any build (javascriptsource, +// deployment/) is left out: it changes without the endpoint too. +func projectFingerprint(t *testing.T, dir string) string { + t.Helper() + var lines []string + add := func(path string) { + b, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + rel, _ := filepath.Rel(dir, path) + lines = append(lines, fmt.Sprintf("%s %x", rel, sha256.Sum256(b))) + } + add(filepath.Join(dir, "App.mpr")) + for _, sub := range []string{"mprcontents", "themesource/mxtest", "javasource/mxtest"} { + root := filepath.Join(dir, sub) + if _, err := os.Stat(root); err != nil { + continue + } + _ = filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + if err == nil && !d.IsDir() { + add(p) + } + return nil + }) + } + return strings.Join(lines, "\n") +} + +func diffFingerprints(before, after string) string { + in := func(s string) map[string]bool { + m := map[string]bool{} + for _, l := range strings.Split(s, "\n") { + m[l] = true + } + return m + } + b, a := in(before), in(after) + var out []string + for l := range b { + if !a[l] { + out = append(out, "- "+l) + } + } + for l := range a { + if !b[l] { + out = append(out, "+ "+l) + } + } + return strings.Join(out, "\n") +} diff --git a/cmd/mxcli/cmd_test_attach_bundle_integration_test.go b/cmd/mxcli/cmd_test_attach_bundle_integration_test.go new file mode 100644 index 0000000000..efaa08fe2e --- /dev/null +++ b/cmd/mxcli/cmd_test_attach_bundle_integration_test.go @@ -0,0 +1,115 @@ +//go:build integration && linux + +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "bytes" + "fmt" + "net/http" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/cmd/mxcli/docker" +) + +// TestTestAttach_KeepsTheHostingAppsPageBundles: `mxcli test --attach` builds +// its test microflows into the app `run --local --test-endpoint` is serving. +// That build went to the run's own mxbuild --serve, where on the prebuilt-client +// Mendix versions (11.12.5+) any build after a process's first leaves web/dist +// without its page modules (mendixlabs/mxcli#1580) — so after a green attach +// the app answered 404 on dist/pages/*.js and every page showed the generic +// error dialog until it was restarted (ako/mxcli#1917, mendixlabs/mxcli#1723). +// +// MXCLI_RUNLIFE_VERSION=11.12.6 go test -tags integration ./cmd/mxcli -run KeepsTheHostingApps -v +// +// The control is the same page module before the attach, which the boot's +// first (complete) build serves with a 200. +func TestTestAttach_KeepsTheHostingAppsPageBundles(t *testing.T) { + version := os.Getenv("MXCLI_RUNLIFE_VERSION") + if version == "" { + version = "11.12.6" + } + mxPath, err := docker.ResolveMxForVersion("", version) + if err != nil { + t.Skipf("mxbuild %s is not cached: %v", version, err) + } + bin := os.Getenv("MXCLI_BIN") + if bin == "" { + bin = filepath.Join(t.TempDir(), "mxcli") + build := exec.Command("go", "build", "-o", bin, ".") + if out, err := build.CombinedOutput(); err != nil { + t.Fatalf("go build: %v\n%s", err, out) + } + } + work, err := os.MkdirTemp("", "mxta") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.RemoveAll(work) }) + create := exec.Command(mxPath, "create-project") + create.Dir = work + if out, err := create.CombinedOutput(); err != nil { + t.Skipf("mx create-project: %v\n%s", err, out) + } + mpr := filepath.Join(work, "App.mpr") + + const appPort, adminPort, servePort = 8487, 8497, 6943 + for _, p := range []int{appPort, adminPort, servePort} { + if portAnswers(p) { + t.Skipf("port %d is in use", p) + } + } + mxcli := func(args ...string) string { + cmd := exec.Command(bin, args...) + cmd.Dir = work + var out bytes.Buffer + cmd.Stdout, cmd.Stderr = &out, &out + if err := cmd.Run(); err != nil { + t.Fatalf("mxcli %v: %v\n%s", args, err, out.String()) + } + return out.String() + } + t.Cleanup(func() { _ = exec.Command(bin, "run", "stop", "-p", mpr).Run() }) + + mxcli("run", "--local", "--test-endpoint", "--detach", "--db-type", "hsqldb", + "--app-port", fmt.Sprint(appPort), "--admin-port", fmt.Sprint(adminPort), + "--serve-port", fmt.Sprint(servePort), "-p", mpr) + + pages, _ := filepath.Glob(filepath.Join(work, "deployment", "web", "dist", "pages", "*.js")) + if len(pages) == 0 { + t.Skip("this Mendix version does not prebuild page modules into web/dist; nothing to lose") + } + pageURL := fmt.Sprintf("http://127.0.0.1:%d/dist/pages/%s", appPort, filepath.Base(pages[0])) + status := func() int { + resp, err := http.Get(pageURL) + if err != nil { + t.Fatalf("GET %s: %v", pageURL, err) + } + resp.Body.Close() + return resp.StatusCode + } + if got := status(); got != http.StatusOK { + t.Fatalf("control: %s answers %d before the attach, want 200", pageURL, got) + } + + test := filepath.Join(work, "trivial.test.mdl") + src := "mdl 1;\n\n/**\n * @test Trivial assertion\n * @expect $Ok = true\n */\ndeclare $Ok boolean = true;\n/\n" + if err := os.WriteFile(test, []byte(src), 0o644); err != nil { + t.Fatal(err) + } + out := mxcli("test", test, "-p", mpr, "--attach", "--skip-app-startup", "--require-assertions") + if !strings.Contains(out, "Passed: 1") { + t.Fatalf("the attach run did not pass:\n%s", out) + } + + if got := status(); got != http.StatusOK { + left, _ := filepath.Glob(filepath.Join(work, "deployment", "web", "dist", "pages", "*.js")) + t.Errorf("%s answers %d after test --attach, want 200 (page modules left in dist/pages: %d)\nattach said:\n%s", + pageURL, got, len(left), out) + } +} diff --git a/cmd/mxcli/docker/serve_attached.go b/cmd/mxcli/docker/serve_attached.go new file mode 100644 index 0000000000..f53d85c90d --- /dev/null +++ b/cmd/mxcli/docker/serve_attached.go @@ -0,0 +1,123 @@ +// SPDX-License-Identifier: Apache-2.0 + +package docker + +import ( + "fmt" + "io" + "net" + "path/filepath" +) + +// AttachedServe builds a project through the mxbuild --serve of a dev loop this +// process does not own — `mxcli test --attach` against `run --local +// --test-endpoint` — with the recovery the dev loop's own builds get. +// +// The attach's builds are never a serve process's first, and on the +// prebuilt-client Mendix versions only the first is complete: any later one +// leaves web/dist without its page modules, and the running app answers 404 on +// every page (mendixlabs/mxcli#1580). The dev loop recovers by restarting its +// serve process. The attach must not: that process belongs to the other mxcli +// and is the one its next build will use. So the attach's "restart" starts a +// serve process of its own on a free port, builds there, and stops it on Close +// (ako/mxcli#1917, mendixlabs/mxcli#1723). +type AttachedServe struct { + rec *serveRecovery + proc *borrowedServe +} + +// NewAttachedServe builds through host, falling back to a private serve process +// for projectPath when a build on host leaves the client bundle incomplete. +func NewAttachedServe(host *ServeServer, projectPath string, out io.Writer) *AttachedServe { + proc := &borrowedServe{ + host: host, + start: func() (ownedServe, error) { return startPrivateServe(projectPath, out) }, + out: out, + } + return &AttachedServe{ + rec: &serveRecovery{serve: proc, deployDir: filepath.Join(filepath.Dir(projectPath), "deployment"), out: out}, + proc: proc, + } +} + +// Build runs one build, recovering as the dev loop's own builds do. +func (a *AttachedServe) Build(req BuildRequest) (*BuildResult, error) { return a.rec.Build(req) } + +// Close stops the private serve process, if one was started. The host's is +// never touched. +func (a *AttachedServe) Close() { + if a.proc.own != nil { + _ = a.proc.own.Stop() + a.proc.own = nil + } +} + +// ownedServe is the part of *ServeServer a private serve process needs — a seam +// so the borrowing is testable without an mxbuild. +type ownedServe interface { + serveProcess + Stop() error +} + +// borrowedServe is the serveProcess under AttachedServe's recovery: builds go to +// the host until a restart is asked for, and from then on to a process of its +// own, which a later restart replaces. +type borrowedServe struct { + host serveProcess + own ownedServe + start func() (ownedServe, error) + out io.Writer +} + +func (b *borrowedServe) Build(req BuildRequest) (*BuildResult, error) { + if b.own != nil { + return b.own.Build(req) + } + return b.host.Build(req) +} + +func (b *borrowedServe) Restart() error { + if b.own != nil { + return b.own.Restart() + } + fmt.Fprintln(b.out, " starting a separate mxbuild --serve for this run (the app's own is left as it is)...") + own, err := b.start() + if err != nil { + return err + } + b.own = own + return nil +} + +// startPrivateServe starts mxbuild --serve for projectPath on a free loopback +// port, resolved the way `run --local` resolves its own. +func startPrivateServe(projectPath string, out io.Writer) (ownedServe, error) { + reader, err := openReadOnly(projectPath) + if err != nil { + return nil, fmt.Errorf("reading the project version: %w", err) + } + pv := reader.ProjectVersion() + reader.Disconnect() + if pv == nil { + return nil, fmt.Errorf("the project has no version") + } + mxbuildPath, err := ResolveMxBuildForLocal("", pv.ProductVersion, out) + if err != nil { + return nil, err + } + port, err := freeLoopbackPort() + if err != nil { + return nil, err + } + javaMajor, _ := ProjectJavaMajor(projectPath) + return StartServe(serveOptionsFor(mxbuildPath, pv.ProductVersion, javaMajor, port)) +} + +func freeLoopbackPort() (int, error) { + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + return 0, fmt.Errorf("finding a free port for mxbuild --serve: %w", err) + } + defer l.Close() + return l.Addr().(*net.TCPAddr).Port, nil +} diff --git a/cmd/mxcli/docker/serve_attached_test.go b/cmd/mxcli/docker/serve_attached_test.go new file mode 100644 index 0000000000..305bbe6690 --- /dev/null +++ b/cmd/mxcli/docker/serve_attached_test.go @@ -0,0 +1,129 @@ +// SPDX-License-Identifier: Apache-2.0 + +package docker + +import ( + "io" + "os" + "path/filepath" + "testing" +) + +// hostServe is the dev loop's serve process as an attach sees it: every build +// is a later build in that process, so it leaves the prebuilt bundle without +// pages. Restarting it is not the attach's to do. +type hostServe struct { + t *testing.T + deploy string + builds int +} + +func (h *hostServe) Build(BuildRequest) (*BuildResult, error) { + h.builds++ + writeIncompleteBundle(h.t, h.deploy) + return buildOK(false), nil +} + +func (h *hostServe) Restart() error { + h.t.Fatal("the attach restarted the dev loop's mxbuild --serve, which it does not own") + return nil +} + +// privateServe is a serve process the attach started: its first build is +// complete, a later one is not, as with any serve process. +type privateServe struct { + scriptedServe + stops int +} + +func (p *privateServe) Stop() error { p.stops++; return nil } + +func newPrivateServe(t *testing.T, deploy string) *privateServe { + return &privateServe{scriptedServe: scriptedServe{build: func(_, nth int) (*BuildResult, error) { + if nth == 1 { + writeCompleteBundle(t, deploy) + return buildOK(true), nil + } + writeIncompleteBundle(t, deploy) + return buildOK(false), nil + }}} +} + +func attachedServeFor(host serveProcess, deploy string, start func() (ownedServe, error)) *AttachedServe { + proc := &borrowedServe{host: host, start: start, out: io.Discard} + return &AttachedServe{rec: &serveRecovery{serve: proc, deployDir: deploy, out: io.Discard}, proc: proc} +} + +func pagesIn(t *testing.T, deploy string) int { + t.Helper() + pages, _ := filepath.Glob(filepath.Join(deploy, "web", "dist", "pages", "*.js")) + return len(pages) +} + +// ako/mxcli#1917: `test --attach` built through the dev loop's serve process, +// whose later builds leave dist without pages, and the app answered 404 on +// every page afterwards. The build now completes the bundle on a serve process +// of the attach's own, and leaves the dev loop's alone. +func TestAttachedServe_CompletesTheBundleOnItsOwnServe(t *testing.T) { + deploy := prebuiltDeployment(t) + host := &hostServe{t: t, deploy: deploy} + var own *privateServe + started := 0 + a := attachedServeFor(host, deploy, func() (ownedServe, error) { + started++ + own = newPrivateServe(t, deploy) + return own, nil + }) + + res, err := a.Build(BuildRequest{ProjectFilePath: "/p/App.mpr"}) + if err != nil || !res.OK() { + t.Fatalf("Build = %+v, %v", res, err) + } + if pagesIn(t, deploy) == 0 { + t.Fatal("dist/pages is empty after the attach's build — the hosting app 404s on every page") + } + if host.builds != 1 || started != 1 { + t.Fatalf("host builds = %d, private serves started = %d; want the host tried once, then one private serve", host.builds, started) + } + if res.RestartRequired { + t.Fatal("RestartRequired must be the incremental build's answer, not the cold one's") + } + + // The cleanup build lands on the private serve, where it is a later build + // too: recovered by restarting the private serve, never the host's. + if res, err := a.Build(BuildRequest{ProjectFilePath: "/p/App.mpr"}); err != nil || !res.OK() { + t.Fatalf("second Build = %+v, %v", res, err) + } + if pagesIn(t, deploy) == 0 { + t.Fatal("dist/pages is empty after the attach's cleanup build") + } + if host.builds != 1 || own.restarts != 1 { + t.Fatalf("host builds = %d, private restarts = %d; want the cleanup on the private serve, restarted once", host.builds, own.restarts) + } + + a.Close() + if own.stops != 1 { + t.Fatalf("private serve stopped %d times on Close, want 1", own.stops) + } +} + +// Control: a host build that leaves the bundle complete is all there is — no +// private serve process is started for it. +func TestAttachedServe_CompleteHostBuildStartsNothing(t *testing.T) { + deploy := prebuiltDeployment(t) + host := &scriptedServe{build: func(_, _ int) (*BuildResult, error) { + writeCompleteBundle(t, deploy) + return buildOK(false), nil + }} + a := attachedServeFor(host, deploy, func() (ownedServe, error) { + t.Fatal("started a private serve process for a complete build") + return nil, nil + }) + if res, err := a.Build(BuildRequest{}); err != nil || !res.OK() { + t.Fatalf("Build = %+v, %v", res, err) + } + if _, err := os.Stat(filepath.Join(deploy, "web", "dist", "widgets.css")); err != nil { + t.Fatal(err) + } + a.Close() // nothing to stop +} diff --git a/cmd/mxcli/marketplace/update.go b/cmd/mxcli/marketplace/update.go index 677a2adc3b..5ac6e7415c 100644 --- a/cmd/mxcli/marketplace/update.go +++ b/cmd/mxcli/marketplace/update.go @@ -5,6 +5,7 @@ package marketplace import ( "archive/zip" "encoding/xml" + "errors" "fmt" "io" "os" @@ -192,6 +193,14 @@ type SkippedFile struct { func PerformUpdate(mprPath, referenceMpr, targetMpk, moduleName, fromVersion, toVersion, toVersionID string, newBackend func() backend.FullBackend) (*UpdateResult, error) { + // Everything below the drop is only worth starting if it can finish. The + // inputs it reads after the drop are checked first, so a missing or + // unreadable one stops the update with the project untouched rather than + // half-way, without the module it was updating (ako/mxcli#1934). + if err := preflightUpdate(referenceMpr, targetMpk, moduleName); err != nil { + return nil, fmt.Errorf("%w; %w", err, ErrUpdateNotStarted) + } + ids, err := CaptureIdentities(mprPath, moduleName) if err != nil { return nil, fmt.Errorf("record the identities the database keys on: %w", err) @@ -248,6 +257,38 @@ func PerformUpdate(mprPath, referenceMpr, targetMpk, moduleName, fromVersion, to }, nil } +// ErrUpdateNotStarted marks a PerformUpdate failure that stopped before the +// first write: the project is as it was, so the caller must not tell the user it +// may be mid-update. +var ErrUpdateNotStarted = errors.New("nothing was changed") + +// preflightUpdate checks the two inputs PerformUpdate reads after it has +// dropped the installed module: the reference project must open and hold the +// module, and the package its bundled files come from must open. +func preflightUpdate(referenceMpr, targetMpk, moduleName string) error { + src, err := modelsdk.Open(referenceMpr) + if err != nil { + return fmt.Errorf("open the new version's reference project %s: %w", referenceMpr, err) + } + defer src.Disconnect() + units, err := src.ListUnits() + if err != nil { + return fmt.Errorf("list the reference project's units: %w", err) + } + moduleUnits, err := unitsOfModule(src, units, moduleName) + if err != nil { + return err + } + if len(moduleUnits) == 0 { + return fmt.Errorf("the new version's reference project has no module %s", moduleName) + } + zr, err := zip.OpenReader(targetMpk) + if err != nil { + return fmt.Errorf("open the new version's package %s: %w", filepath.Base(targetMpk), err) + } + return zr.Close() +} + // StampMarketplaceVersion records which published version a module now is. // // Necessary because the reference project is built with `mx module-import`, diff --git a/cmd/mxcli/marketplace/update_preflight_test.go b/cmd/mxcli/marketplace/update_preflight_test.go new file mode 100644 index 0000000000..3460c91a81 --- /dev/null +++ b/cmd/mxcli/marketplace/update_preflight_test.go @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: Apache-2.0 + +package marketplace + +import ( + "crypto/sha256" + "fmt" + "io/fs" + "os" + "path/filepath" + "sort" + "strings" + "testing" +) + +// projectDigest hashes every file under the project's directory, so a test can +// tell whether anything at all was written. +func projectDigest(t *testing.T, mpr string) string { + t.Helper() + root := filepath.Dir(mpr) + var lines []string + err := filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + if err != nil || d.IsDir() { + return err + } + b, err := os.ReadFile(p) + if err != nil { + return err + } + rel, _ := filepath.Rel(root, p) + lines = append(lines, fmt.Sprintf("%s %x", rel, sha256.Sum256(b))) + return nil + }) + if err != nil { + t.Fatal(err) + } + sort.Strings(lines) + return strings.Join(lines, "\n") +} + +// ako/mxcli#1917's neighbour, mendixlabs/mxcli#1732 (ako/mxcli#1934): +// `marketplace update` dropped the installed module, copied in the new one, and +// only then opened the downloaded package — which was not where it was looked +// for — so every update failed half-way and left the project without the module +// it was meant to update. An input the update cannot use must stop it before +// the first write. +func TestPerformUpdate_RefusesBeforeDroppingWhenAnInputIsMissing(t *testing.T) { + for name, missing := range map[string]string{ + "package": "package", + "reference project": "reference", + } { + t.Run(name, func(t *testing.T) { + target := copyFixture(t) + ref := copyFixture(t) + mpk := emptyMpk(t) + gone := filepath.Join(t.TempDir(), "absent") + if missing == "package" { + mpk = gone + ".mpk" + } else { + ref = gone + ".mpr" + } + before := projectDigest(t, target) + + _, err := PerformUpdate(target, ref, mpk, "Administration", + "4.3.2", "4.3.3", "00000000-0000-0000-0000-000000000000", testBackend) + if err == nil { + t.Fatal("PerformUpdate succeeded with a missing " + name) + } + if after := projectDigest(t, target); after != before { + t.Fatalf("the project was changed by an update that could not complete (missing %s): %v", name, err) + } + if !strings.Contains(err.Error(), "nothing was changed") { + t.Errorf("the error should say the project is untouched, got: %v", err) + } + }) + } +} diff --git a/cmd/mxcli/syntax/features_misc.go b/cmd/mxcli/syntax/features_misc.go index ecb762084b..302caf0c5e 100644 --- a/cmd/mxcli/syntax/features_misc.go +++ b/cmd/mxcli/syntax/features_misc.go @@ -1480,6 +1480,30 @@ mxcli check script.mdl -p app.mpr --references -- With reference validation`, -- Fix: Use fully qualified name: Enumeration(MyModule.Status)`, }) + Register(SyntaxFeature{ + Path: "module.settings", + Summary: "A module's version and module type (Studio Pro's Module settings, General tab)", + Keywords: []string{ + "module settings", "module version", "module type", "add-on module", + "addon", "app module", "semver", "export module", + }, + Syntax: `CREATE [OR MODIFY] MODULE ( Version: 'x.y.z', ModuleType: app | addon ); +ALTER MODULE SET ( Version: 'x.y.z', ModuleType: app | addon ); +DESCRIBE MODULE ; -- prints the settings that differ from App / 1.0.0`, + Example: `mdl 1; +-- A versioned add-on module +create or modify module Inventory ( + Version: '2.3.1', + ModuleType: addon, +); + +-- Bump the version before exporting it +alter module Inventory set (Version: '2.4.0'); + +-- An add-on hides elements not marked Usable from other modules (CE1406); +-- check -p --references reports the references that breaks.`, + }) + Register(SyntaxFeature{ Path: "module.jar-dependencies", Summary: "Manage Maven/JAR dependencies in a module's settings", diff --git a/cmd/mxcli/testrunner/host.go b/cmd/mxcli/testrunner/host.go index e6ba9c5629..8e5845ba48 100644 --- a/cmd/mxcli/testrunner/host.go +++ b/cmd/mxcli/testrunner/host.go @@ -43,10 +43,22 @@ func InstallHostedEndpoint(projectPath string, w io.Writer) (*HostedEndpoint, er return nil, err } + // An endpoint an earlier run could not remove (a Windows `run stop`, a crash) + // comes out first: captured as the project's own state, its after-startup + // would be chained to and its module kept as the user's. + if _, err := RestoreAbandonedEndpoint(projectPath, w); err != nil { + return nil, err + } + state, err := captureProjectState(projectPath) if err != nil { return nil, fmt.Errorf("capturing project state: %w", err) } + // Written before anything is installed, so a run that is killed from here on + // can still be undone by `run stop` (ako/mxcli#1917). + if err := writeRestoreRecord(projectPath, os.Getpid(), state); err != nil { + fmt.Fprintf(w, "Warning: could not record how to remove the test endpoint (%v); if this run is killed, `run stop` cannot remove it\n", err) + } h := &HostedEndpoint{ Token: token, @@ -118,5 +130,7 @@ func (h *HostedEndpoint) Remove() { // Only past the error return above, so this never claims a project is // unchanged while an injection is still in it. restoreProjectFile(h.state, nil, h.out) + // Kept on the error path above, so `run stop` tries the removal again. + removeRestoreRecord(h.projectPath) fmt.Fprintln(h.out, " test endpoint removed; project restored") } diff --git a/cmd/mxcli/testrunner/host_restore.go b/cmd/mxcli/testrunner/host_restore.go new file mode 100644 index 0000000000..a19540b1ee --- /dev/null +++ b/cmd/mxcli/testrunner/host_restore.go @@ -0,0 +1,144 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testrunner + +import ( + "encoding/json" + "fmt" + "io" + "os" + "path/filepath" + + "github.com/mendixlabs/mxcli/cmd/mxcli/docker" +) + +// The restore record lets a process other than the dev loop undo what +// --test-endpoint installed. The dev loop removes the endpoint on its own way +// out, but it does not always get one: on Windows `run stop` ends a detached run +// with `taskkill /F`, which no handler sees, and anywhere a crash or a SIGKILL +// skips it too. The project was then left carrying the MxTest module, an +// after-startup pointing at it and themesource/mxtest/, and the next build +// failed with CE1613 once the module was dropped (ako/mxcli#1917, +// mendixlabs/mxcli#1723). +// +// So what Remove needs is written to disk before anything is installed — the +// original after-startup, whether MxTest pre-existed, and the project file as it +// was — and whoever finds the record with its owner gone (`run stop`, or the +// next --test-endpoint run) performs the same removal. +const ( + restoreRecordName = "test-endpoint-restore.json" + restoreMprName = "test-endpoint-restore.mpr" +) + +type restoreRecord struct { + // PID is the dev loop that installed the endpoint. While it is alive the + // endpoint is its to remove. + PID int `json:"pid"` + AfterStartup string `json:"afterStartup"` + CreatedMxTest bool `json:"createdMxTest"` + // TreeDigest pairs with the saved project file, which is only put back while + // the document tree still matches it (see projectSnapshot.restore). + TreeDigest string `json:"treeDigest,omitempty"` +} + +func restoreRecordPath(projectPath string) string { + return filepath.Join(filepath.Dir(projectPath), ".mxcli", restoreRecordName) +} + +func restoreMprPath(projectPath string) string { + return filepath.Join(filepath.Dir(projectPath), ".mxcli", restoreMprName) +} + +// writeRestoreRecord saves st for pid. The project-file copy is written before +// the record that refers to it, and the record through a rename, so a reader +// never finds a record whose copy is missing or half written. +func writeRestoreRecord(projectPath string, pid int, st projectState) error { + dir := filepath.Dir(restoreRecordPath(projectPath)) + if err := os.MkdirAll(dir, 0o755); err != nil { + return err + } + rec := restoreRecord{PID: pid, AfterStartup: st.afterStartup, CreatedMxTest: st.createdMxTest} + if st.snapshot.path != "" { + if err := os.WriteFile(restoreMprPath(projectPath), st.snapshot.contents, 0o600); err != nil { + return err + } + rec.TreeDigest = st.snapshot.treeDigest + } + body, err := json.MarshalIndent(rec, "", " ") + if err != nil { + return err + } + tmp := restoreRecordPath(projectPath) + ".tmp" + if err := os.WriteFile(tmp, body, 0o600); err != nil { + return err + } + return os.Rename(tmp, restoreRecordPath(projectPath)) +} + +// readRestoreRecord returns the record and the project state it describes, or +// nil when there is none. +func readRestoreRecord(projectPath string) (*restoreRecord, projectState, error) { + body, err := os.ReadFile(restoreRecordPath(projectPath)) + if os.IsNotExist(err) { + return nil, projectState{}, nil + } + if err != nil { + return nil, projectState{}, err + } + var rec restoreRecord + if err := json.Unmarshal(body, &rec); err != nil { + return nil, projectState{}, fmt.Errorf("reading %s: %w", restoreRecordPath(projectPath), err) + } + st := projectState{afterStartup: rec.AfterStartup, createdMxTest: rec.CreatedMxTest} + if rec.TreeDigest != "" { + // A missing copy only costs the byte-exact restore; the removal itself + // does not depend on it. + if data, err := os.ReadFile(restoreMprPath(projectPath)); err == nil { + st.snapshot = projectSnapshot{path: projectPath, contents: data, treeDigest: rec.TreeDigest} + } + } + return &rec, st, nil +} + +func removeRestoreRecord(projectPath string) { + os.Remove(restoreRecordPath(projectPath)) + os.Remove(restoreMprPath(projectPath)) +} + +// Seams for the tests: the removal shells out to mxcli, which a unit test +// cannot do (findMxcli resolves to the test binary). +var ( + pidAlive = docker.PidAlive + removeInstalled = func(projectPath string, st projectState, w io.Writer) error { + if err := cleanupEndpoint(projectPath, st, &TestSuite{}, w); err != nil { + return err + } + removeGeneratedJavaSource(projectPath, w) + restoreProjectFile(st, nil, w) + return nil + } +) + +// RestoreAbandonedEndpoint removes a test endpoint whose dev loop ended without +// removing it, and reports whether there was one. A record whose owner is still +// running is left alone: that endpoint is in use, and its owner removes it. +// +// On failure the record stays, so the next `run stop` or --test-endpoint run +// tries again, and the error says what is left in the project. +func RestoreAbandonedEndpoint(projectPath string, w io.Writer) (bool, error) { + rec, st, err := readRestoreRecord(projectPath) + if err != nil || rec == nil { + return false, err + } + if rec.PID > 0 && pidAlive(rec.PID) { + return false, nil + } + fmt.Fprintf(w, "Removing the test endpoint left in the project by run pid %d (its run ended without removing it)...\n", rec.PID) + RemoveHandshake(projectPath) + if err := removeInstalled(projectPath, st, w); err != nil { + return true, fmt.Errorf("could not remove the test endpoint — check the after-startup microflow and the %s module before committing: %w", mxTestModule, err) + } + removeRestoreRecord(projectPath) + fmt.Fprintln(w, " test endpoint removed; project restored") + return true, nil +} diff --git a/cmd/mxcli/testrunner/host_restore_test.go b/cmd/mxcli/testrunner/host_restore_test.go new file mode 100644 index 0000000000..af4a1ed3aa --- /dev/null +++ b/cmd/mxcli/testrunner/host_restore_test.go @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testrunner + +import ( + "errors" + "io" + "os" + "path/filepath" + "testing" +) + +// withRestoreSeams replaces the liveness check and the removal for one test. +func withRestoreSeams(t *testing.T, alive bool, remove func(string, projectState, io.Writer) error) { + t.Helper() + oldAlive, oldRemove := pidAlive, removeInstalled + pidAlive = func(int) bool { return alive } + removeInstalled = remove + t.Cleanup(func() { pidAlive, removeInstalled = oldAlive, oldRemove }) +} + +func recordedProject(t *testing.T) string { + t.Helper() + mpr := filepath.Join(t.TempDir(), "App.mpr") + if err := os.WriteFile(mpr, []byte("before"), 0o644); err != nil { + t.Fatal(err) + } + st := projectState{ + afterStartup: "MyFirstModule.ASU", + createdMxTest: true, + snapshot: projectSnapshot{path: mpr, contents: []byte("before"), treeDigest: "d1"}, + } + if err := writeRestoreRecord(mpr, 4242, st); err != nil { + t.Fatal(err) + } + return mpr +} + +// The run that installed the endpoint is gone (taskkill /F, SIGKILL): the +// removal runs with the state it recorded, and the record goes with it. +func TestRestoreAbandonedEndpoint_RemovesWhatADeadRunLeft(t *testing.T) { + mpr := recordedProject(t) + var got projectState + withRestoreSeams(t, false, func(_ string, st projectState, _ io.Writer) error { + got = st + return nil + }) + + restored, err := RestoreAbandonedEndpoint(mpr, io.Discard) + if err != nil || !restored { + t.Fatalf("restored=%v err=%v, want true, nil", restored, err) + } + if got.afterStartup != "MyFirstModule.ASU" || !got.createdMxTest { + t.Errorf("removal got %+v, want the recorded after-startup and createdMxTest", got) + } + if string(got.snapshot.contents) != "before" || got.snapshot.treeDigest != "d1" || got.snapshot.path != mpr { + t.Errorf("removal got snapshot %q/%q/%q, want the saved project file", got.snapshot.contents, got.snapshot.treeDigest, got.snapshot.path) + } + for _, p := range []string{restoreRecordPath(mpr), restoreMprPath(mpr)} { + if _, err := os.Stat(p); !os.IsNotExist(err) { + t.Errorf("%s survives a successful removal", p) + } + } +} + +// Control: an endpoint whose run is still serving is that run's to remove. +func TestRestoreAbandonedEndpoint_LeavesALiveRunsEndpoint(t *testing.T) { + mpr := recordedProject(t) + withRestoreSeams(t, true, func(string, projectState, io.Writer) error { + t.Fatal("removed the endpoint of a run that is still alive") + return nil + }) + if restored, err := RestoreAbandonedEndpoint(mpr, io.Discard); restored || err != nil { + t.Fatalf("restored=%v err=%v, want false, nil", restored, err) + } + if _, err := os.Stat(restoreRecordPath(mpr)); err != nil { + t.Errorf("the live run's record was removed: %v", err) + } +} + +// A failed removal keeps the record, so the next stop or run tries again. +func TestRestoreAbandonedEndpoint_KeepsTheRecordWhenRemovalFails(t *testing.T) { + mpr := recordedProject(t) + withRestoreSeams(t, false, func(string, projectState, io.Writer) error { return errors.New("drop failed") }) + if _, err := RestoreAbandonedEndpoint(mpr, io.Discard); err == nil { + t.Fatal("a failed removal reported success") + } + if _, err := os.Stat(restoreRecordPath(mpr)); err != nil { + t.Errorf("the record was removed although the endpoint is still in the project: %v", err) + } +} + +func TestRestoreAbandonedEndpoint_NoRecordIsNoOp(t *testing.T) { + mpr := filepath.Join(t.TempDir(), "App.mpr") + withRestoreSeams(t, false, func(string, projectState, io.Writer) error { + t.Fatal("removal ran without a record") + return nil + }) + if restored, err := RestoreAbandonedEndpoint(mpr, io.Discard); restored || err != nil { + t.Fatalf("restored=%v err=%v, want false, nil", restored, err) + } +} diff --git a/cmd/mxcli/testrunner/runner_attach.go b/cmd/mxcli/testrunner/runner_attach.go index 800ed11eda..f6e75d6eb2 100644 --- a/cmd/mxcli/testrunner/runner_attach.go +++ b/cmd/mxcli/testrunner/runner_attach.go @@ -17,11 +17,17 @@ import ( // server, so it must never stop them. type attachedApp struct { client *endpointClient - serve *docker.ServeServer + // builds go through the dev loop's serve server, falling back to a serve + // process of the attach's own when that one would leave the client bundle + // without its pages (ako/mxcli#1917). close stops only the attach's own. + builds *docker.AttachedServe ctrl *docker.RuntimeController hs *Handshake } +// close stops what the attach started; the dev loop's processes are not its to stop. +func (a *attachedApp) close() { a.builds.Close() } + // attach connects to an app already hosting the test endpoint. func attach(opts RunOptions, w io.Writer) (*attachedApp, error) { hs, err := ReadHandshake(opts.ProjectPath) @@ -43,7 +49,7 @@ func attach(opts RunOptions, w io.Writer) (*attachedApp, error) { return &attachedApp{ client: client, - serve: &docker.ServeServer{Host: "127.0.0.1", Port: hs.ServePort}, + builds: docker.NewAttachedServe(&docker.ServeServer{Host: "127.0.0.1", Port: hs.ServePort}, opts.ProjectPath, w), // The admin password, not the endpoint token — different secrets. ctrl: docker.NewRuntimeController(docker.M2EEOptions{ Host: "127.0.0.1", @@ -76,7 +82,7 @@ func (a *attachedApp) adminOptions() docker.M2EEOptions { } func (a *attachedApp) applyModelChange(projectPath string) (string, error) { - build, err := a.serve.Build(docker.BuildRequest{Target: docker.TargetDeploy, ProjectFilePath: projectPath}) + build, err := a.builds.Build(docker.BuildRequest{Target: docker.TargetDeploy, ProjectFilePath: projectPath}) if err != nil { return "", fmt.Errorf("rebuilding through the attached app's build server on port %d: %w", a.hs.ServePort, err) } @@ -112,6 +118,7 @@ func runAttached(opts RunOptions, suite *TestSuite, timeout time.Duration, w io. if err != nil { return nil, err } + defer app.close() // Only the generated test microflows are ours to remove. injected := suite diff --git a/docs-site/src/SUMMARY.md b/docs-site/src/SUMMARY.md index 134b0e1b9d..0127601599 100644 --- a/docs-site/src/SUMMARY.md +++ b/docs-site/src/SUMMARY.md @@ -311,6 +311,7 @@ - [ALTER SETTINGS](reference/settings/alter-settings.md) - [Organization Statements](reference/organization/README.md) - [CREATE MODULE](reference/organization/create-module.md) + - [ALTER MODULE … SET](reference/organization/alter-module-set.md) - [CREATE FOLDER](reference/organization/create-folder.md) - [DROP FOLDER](reference/organization/drop-folder.md) - [MOVE](reference/organization/move.md) diff --git a/docs-site/src/reference/organization/alter-module-set.md b/docs-site/src/reference/organization/alter-module-set.md new file mode 100644 index 0000000000..28ce5d38ac --- /dev/null +++ b/docs-site/src/reference/organization/alter-module-set.md @@ -0,0 +1,36 @@ +# ALTER MODULE … SET + +## Synopsis + + ALTER MODULE module_name SET ( Version: 'x.y.z', ModuleType: app | addon ) + +## Description + +Changes a module's settings, the General tab of Studio Pro's *Module settings* dialog. It takes the same keys as [CREATE MODULE](create-module.md), and changes only the settings it names. The rest of the module's settings, including its *Module ID* and an imported module's checksum, keep their stored values. A statement that restates the stored settings writes nothing. + +| Key | Values | Studio Pro field | +|---|---|---| +| `Version` | a SemVer string, `'2.3.1'` | Module version | +| `ModuleType` | `app` \| `addon` | Module type | + +An unknown key, a key named twice, a version that is not SemVer, or a type other than `app`/`addon` is an error from both `check` and `exec`. `ModuleType: addon` needs Mendix 9.12 or later. + +Making a module an add-on hides every element not marked *Usable* from the rest of the app: a reference from another module becomes CE1406. Run `mxcli check script.mdl -p app.mpr --references` to see which references that affects. + +## Examples + +```sql +mdl 1; +-- Bump the version before exporting the module +alter module Inventory set (Version: '2.4.0'); + +-- Make it an add-on module +alter module Inventory set (ModuleType: addon); + +-- Both at once +alter module Inventory set (Version: '3.0.0', ModuleType: app); +``` + +## See Also + +[CREATE MODULE](create-module.md), [ALTER MODULE … JAR DEPENDENCY](../jar-dependency/alter-module-jar-dependency.md) diff --git a/docs-site/src/reference/organization/create-module.md b/docs-site/src/reference/organization/create-module.md index d3ea827a0a..de2c0baf51 100644 --- a/docs-site/src/reference/organization/create-module.md +++ b/docs-site/src/reference/organization/create-module.md @@ -2,17 +2,42 @@ ## Synopsis - CREATE MODULE module_name + CREATE [OR MODIFY] MODULE module_name [ ( Version: 'x.y.z', ModuleType: app | addon ) ] ## Description -Creates a new module in the Mendix project. Modules are the top-level organizational unit for all project documents including entities, microflows, pages, and enumerations. The module is created with default settings and an empty domain model. +Creates a new module in the Mendix project. Modules are the top-level organizational unit for all project documents including entities, microflows, pages, and enumerations. The module is created with an empty domain model and Studio Pro's defaults for a new module: an **app module**, version **1.0.0**. + +The optional property list sets the module's settings, the General tab of Studio Pro's *Module settings* dialog. `CREATE OR MODIFY` applies them to a module that already exists, changing only the settings it names. A plain `CREATE` of an existing module leaves it unchanged. To change one setting, use [ALTER MODULE … SET](alter-module-set.md). ## Parameters **module_name** : The name of the module to create. Must be a valid identifier (letters, digits, underscores; cannot start with a digit). If the name conflicts with a reserved keyword, enclose it in double quotes. +**Version** +: The *Module version*, a SemVer string `'major.minor.patch'` with an optional pre-release or build suffix (`'1.0.0-rc.1'`). It is the version an exported `.mpk` carries. Anything else is refused, as the dialog refuses it. + +**ModuleType** +: `app` (an *App module*, the default) or `addon` (an *Add-on module*). An add-on hides every element not marked *Usable* from the rest of the app: referencing one from another module is CE1406, which `check -p --references` reports. Add-on modules need Mendix 9.12 or later. Solution modules have no MDL spelling. + +## Settings shown by DESCRIBE MODULE + +`DESCRIBE MODULE` prints the module's `CREATE OR MODIFY` statement with the settings that differ from a new module's, so its output re-runs: + +```sql +create or modify module Inventory ( + Version: '2.3.1', + ModuleType: addon, +); +-- location: Marketplace modules (set in Studio Pro) +-- add-on module: elements not marked Usable cannot be referenced from other modules +``` + +The *Location* (Module list or Marketplace modules) is printed as a comment when it is *Marketplace modules*; it cannot be set from MDL yet. The *Module ID* and the checksum of an imported module are not shown: Studio Pro owns them, and setting a module's settings keeps them. + +`LIST MODULES` shows each module's version and type in its `Version` and `Type` columns. + ## Examples ### Create a module @@ -30,6 +55,16 @@ CREATE MODULE Inventory; CREATE MODULE Reporting; ``` +### Create a versioned add-on module + +```sql +mdl 1; +create or modify module Inventory ( + Version: '2.3.1', + ModuleType: addon, +); +``` + ## See Also -[CREATE FOLDER](create-folder.md), [MOVE](move.md), [DROP MODULE](../domain-model/drop-entity.md) +[ALTER MODULE … SET](alter-module-set.md), [CREATE FOLDER](create-folder.md), [MOVE](move.md), [DROP MODULE](../domain-model/drop-entity.md) diff --git a/docs-wiki/bug-patterns/keyword-collisions.md b/docs-wiki/bug-patterns/keyword-collisions.md index 4ab8b92a84..f737ac3d8f 100644 --- a/docs-wiki/bug-patterns/keyword-collisions.md +++ b/docs-wiki/bug-patterns/keyword-collisions.md @@ -103,6 +103,19 @@ it either, since ANTLR's recovery consumed the stray bracket up to EOF and the EOF check passed. When relaxing a rule to accept repetition, the AST, every serializer and every rewriter change with it. +**A multi-word token collides in pairs, which no keyword check sees.** The +`error message` clause is one lexer token, so an attribute named `Error` +followed by the `message` keyword (`validation feedback $E/Error message 'x'`, +verbatim describe output) lexed as the clause. Neither word is a problem alone, +so the lexer-driven quoting in describe passes `Error` unquoted. Fix it where the +token is made: a lexer predicate on context (here "not after a `/` or `.` +separator") covers every rule at once, but walking backwards over line breaks +lets a comment ending in `.` or `/` pass for a separator. Put the comment cases in +as controls. **Put the predicate on the right edge.** ANTLR evaluates a left-edge +lexer predicate at every token start, which turns off the lexer's DFA cache. Here +that doubled `check` time and timed out the corpus tests, while every unit test +passed. Time a lexer change against a control binary. + **Quoting is not one question.** Quoting escapes MDL's *parser* keywords. It does not escape Mendix's platform-reserved member names, and it does not help in an OQL alias. Three vocabularies stack, and a rule that conflates them produces diff --git a/docs/11-proposals/PROPOSAL_module_settings.md b/docs/11-proposals/PROPOSAL_module_settings.md new file mode 100644 index 0000000000..dfe5491a4c --- /dev/null +++ b/docs/11-proposals/PROPOSAL_module_settings.md @@ -0,0 +1,255 @@ +--- +title: Module settings — describe and set the module version and module type +status: partial +date: 2026-11-03 +author: Generated with Claude Code +issue: ako/mxcli#1921 +supersedes: show-describe-module-settings.md +depends-on: ako/mxcli#1920 +--- + +# Proposal: Module settings — describe and set the module version and module type + +**Status:** Partial — Version and ModuleType implemented (ako/mxcli#1921); Location is the later step. +**Date:** 2026-11-03 + +## Problem Statement + +The General tab of Studio Pro's *Module settings* dialog has four fields: +*Module type* (App module / Add-on module), *Module version*, *Location* +(Module list / Marketplace modules) and *Module ID*. mxcli can describe none of +them and set none of them. `describe module Odata` prints + +```mdl +create or modify module Odata; +create or modify module role Odata.User; +``` + +plus, since #1498, a comment when the module is Protected. Only the *Java +Dependencies* tab has syntax (`alter module … jar dependency`). + +Who benefits: + +- **Teams versioning their own modules.** A module meant for reuse is exported as + an `.mpk` with its version, and the version is what a consumer sees when the + module is updated. Scripts that build or refactor such a module cannot bump the + version today, and `describe` cannot tell an agent which version it is looking + at. +- **Authors of add-on modules.** An add-on module hides every element not marked + *Usable* from the rest of the app (CE1406). Making a module an add-on from MDL + lets a script build it complete, and `describe` lets an agent see why a + reference is refused. + +This supersedes [`show-describe-module-settings.md`](show-describe-module-settings.md), +a read-only proposal from before the modelsdk engine. Its BSON notes are partly +wrong (`version` lowercase, `ProtectedModuleType: "none"`, neither of which +Studio Pro stores). + +## BSON Structure + +Measured on ako/TestApp (Mendix 11.16.0 Beta): 24 modules, plus one probe module, +`ModSettingsProbe`, changed one field at a time in Studio Pro and diffed between +saves. + +### `Projects$ModuleSettings` — one unit per module, contained by the module + +| Key | Observed values | Meaning | +|---|---|---| +| `ExportLevel` | `Source` (every app module), `Protected` (the probe, set to *Add-on module*) | **The module type.** `Source` = App module, `Protected` = Add-on module. | +| `ProtectedModuleType` | `AddOn` on **every** module, app modules included | Which kind of protected module; only meaningful when `ExportLevel` is `Protected`. Metamodel literals `AddOn`, `Solution`; introduced 9.12.0. | +| `Version` | `1.0.0` (every untouched app module), `1.0.1`, `2.3.1` (probe), `4.11.0` (WorkflowCommons) | *Module version*, a plain string; the dialog requires SemVer. | +| `PackageId` | a UUID per module | *Module ID*, shown read-only with an edit pencil. | +| `Checksum` | empty, or base64 on an imported Marketplace module | Studio Pro uses it to tell whether an imported module was modified. | +| `ConvertedChecksum`, `OriginalPackageId`, `BasedOnVersion`, `SolutionIdentifier`, `ExtensionName` | empty everywhere | — | +| `EnableDetailedTroubleshooting` | `true` or `false` | Not on the General tab. | +| `ModuleDependencies` | `null` everywhere | — | +| `JarDependencies` | marker-2 list | Already supported. | + +**What a measured edit changed:** + +| Dialog edit | Stored change | +|---|---| +| Module version 1.0.1 → 2.3.1 | `ModuleSettings.Version` only | +| Location → Marketplace modules | `Projects$ModuleImpl.FromAppStore` `false` → `true`; `AppStoreGuid`, `AppStoreVersion`, `AppStoreVersionGuid` and `AppStorePackageIdString` stay empty | +| Module type → Add-on | `ExportLevel` `Source` → `Protected`, and nothing else (confirmed by the maintainer in Studio Pro 11.16) | + +### `Projects$ModuleImpl` — the module itself + +`Name`, `FromAppStore`, `AppStoreGuid`, `AppStoreVersion`, `AppStoreVersionGuid`, +`AppStorePackageIdString`, `IsThemeModule`, `NewSortIndex`. *Location* is +`FromAppStore`. `show modules` already reads `AppStoreVersion` for its `Source` +column (`Marketplace v4.11.0`). A module moved to *Marketplace modules* by hand +has `FromAppStore: true` and **no** App Store version. + +### Gotchas + +- **`ProtectedModuleType` does not say "add-on".** Every app module stores + `AddOn` there, so reading it alone makes every module look like an add-on. The + type is `ExportLevel`. +- **Writing the settings unit must carry what mxcli does not model.** Before + #1920, the only writer rebuilt the unit and wiped `PackageId` and `Checksum`. + This proposal adds writes to the same unit, so it depends on #1920's carry. +- **Location and the Marketplace fields are one concept to Studio Pro.** + `FromAppStore` is also how a real Marketplace import is recognised, so writing + it is a separate step with its own measurement (see *Later: Location*). + +## Proposed MDL Syntax + +The module's settings are properties of the `create` statement (R2), and +`alter … set` takes exactly the same keys (R3). Keys are named after the dialog's +fields. + +```mdl +-- Create or update a module with its settings +create or modify module Inventory ( + Version: '2.3.1', + ModuleType: addon, +); + +-- Change one setting +alter module Inventory set (Version: '2.4.0'); +alter module Inventory set (ModuleType: app); +``` + +| Key | Values | Stored as | Default (omitted by `describe`, R12) | +|---|---|---|---| +| `Version` | a SemVer string, `'major.minor.patch'` | `ModuleSettings.Version` | `'1.0.0'` | +| `ModuleType` | `app` \| `addon` | `ExportLevel` `Source` \| `Protected`, with `ProtectedModuleType` `AddOn` | `app` | + +`describe module` prints the same statement with only the settings that differ +from the defaults, so its output re-runs (R6): + +```mdl +create or modify module ModSettingsProbe ( + Version: '2.3.1', + ModuleType: addon, +); +-- location: Marketplace modules (set in Studio Pro) +``` + +**Read-only for now, printed as comments:** + +- **Location**, only when it is *Marketplace modules*: `-- location: Marketplace + modules (set in Studio Pro)`. A real import already shows its App Store version + in `show modules`. It becomes a key in a later step (*Later: Location*), and the + comment then turns into `Location: marketplace`. +- The **#1498 comment** about elements not marked *Usable* stays, attached to the + `ModuleType: addon` line it explains, instead of standing alone. + +**Not shown:** *Module ID* (`PackageId`) and the checksum. They are identifiers +Studio Pro owns, and printing them adds a line to every module's description +without anything a script could do with it. + +**Validation** (shared by `check` and `exec`, so they cannot drift): + +- `Version` must be SemVer `major.minor.patch` with an optional pre-release + suffix. Studio Pro refuses anything else in the dialog. +- An unknown key, a repeated key, or a `ModuleType` other than `app`/`addon` is + an error from the visitor (R11). +- `ModuleType: addon` below Mendix 9.12 is refused (MDL-VERSION01), the release + that introduced `ProtectedModuleType`. +- `ModuleType: solution` is not accepted until a solution project has been + measured (see Open Questions). + +`show modules` gains two columns, `Version` and `Type`, so a project's module +versions can be listed in one command. + +### Later: Location + +*Location* is meant to become writable, as a third key with the same shape: + +```mdl +alter module Inventory set (Location: marketplace); -- or: modulelist +``` + +It writes `Projects$ModuleImpl.FromAppStore`, a different unit from the other two +keys. It is not in the first implementation, because a hand-made module set to +*Marketplace modules* and a real import both have `FromAppStore: true`, and only +the import carries an App Store GUID and version. Before the key lands, measure: + +- whether Studio Pro changes anything else when the location is switched, and + back again; +- whether moving a **real import** to *Module list* clears or keeps its App Store + fields, since those fields are what `show modules` and the Marketplace upgrade + (`PROPOSAL_marketplace_module_upgrade.md`) recognise an import by. + +Until then the comment form stays, and `describe` output re-runs unchanged. + +## Implementation Plan + +Order matters: the write path lands only on top of #1920, and `describe` lands +before the syntax, so the read side can be verified on real projects first. + +1. **Read (describe + show).** `describe module` prints `Version` and + `ModuleType` when they differ from the defaults, and the Location comment. + `show modules` gains the two columns. No grammar change. +2. **Grammar + visitor.** `createModuleStatement` takes a `( Key: value, … )` + list after the name (the existing `folder '…'` option stays a clause, R9), and + a new `alterModuleSetStatement` takes `set ( … )`. Key table and value checks + in the visitor, following `visitor_rest_call_settings.go`. +3. **Executor + backend.** `execCreateModule` (create and `create or modify`) and + a new `execAlterModuleSet` write through `UpdateModuleSettings`, which (after + #1920) writes onto the stored unit and changes only the keys MDL names. + `CreateModule` writes the stated settings for a new module. +4. **Check.** SemVer and version-gate validation in the shared check path. + +### Files to modify/create + +| File | Change | +|---|---| +| `mdl/executor/cmd_modules.go` | `describeModule`: settings in the create statement, Location comment; `show modules` columns; `execCreateModule` applies settings; new `execAlterModuleSet` | +| `mdl/grammar/domains/MDLDomainModel.g4` | property list on `createModuleStatement`; `alterModuleSetStatement` | +| `mdl/grammar/MDLParser.g4` | register `alterModuleSetStatement` beside `alterModuleJarDepStatement` | +| `mdl/ast/ast_enumeration.go`, `mdl/ast/ast_module_settings.go` | `CreateModuleStmt.Settings`; `AlterModuleSetStmt` | +| `mdl/visitor/visitor_module_settings.go` *(new)* | key table, value parsing, unknown-key errors | +| `mdl/types/module_settings.go` | no new fields: `Version` and `ExportLevel` are already there | +| `mdl/backend/modelsdk/module_settings_write.go` | none beyond #1920; `CreateModule` takes the stated settings | +| `mdl/backend/modelsdk/module_write.go` | `CreateModule` writes the stated `Version` / `ExportLevel` | +| `mdl/executor/` check path | SemVer and 9.12 gate, shared by `check` and `exec` | +| `docs-site/src/` (modules page), `mxcli syntax module` | the new keys | +| `.claude/skills/mendix/` (module skill, if one covers modules) | `ModuleType: addon` and the *Usable* consequence | + +## Version Compatibility + +- `Version` and `ExportLevel` are in every supported Mendix version's + metamodel: no gate for `Version`. +- `ModuleType: addon` writes `ProtectedModuleType`, introduced in 9.12.0, and is + refused below it. +- The MCP backend has no module-settings write (`unsupported_gen.go`). It returns + its usual unsupported error, the same as `alter module … jar dependency` today. + +## Test Plan + +- **Fixture-backed read tests:** the probe module's settings and module units + (Add-on, 2.3.1, Marketplace location) and an untouched app module's, from + ako/TestApp. `describe` prints the expected statement and comment for each. + Control: the app module prints `create or modify module Odata;` alone. +- **Round trip:** describe → exec on a copy writes nothing (elision) for the + probe, with a control edit (`Version: '2.3.2'`) that does write, so the + "nothing changed" assertion is not vacuous. +- **Write tests:** `alter module … set (Version: …)` and `set (ModuleType: addon)` + change exactly `Version` / `ExportLevel`, and every other key of the + Studio Pro unit stays byte-identical (the #1920 test, extended). +- **Validation:** `check` refuses `Version: '2.3'`, `ModuleType: solution`, an + unknown key, and `addon` under a 9.11 project version, and `exec` refuses the + same, with no write. +- **Studio Pro check:** open a project where `alter module … set (ModuleType: + addon)` ran, confirm the dialog shows *Add-on module* and `mx check` reports no + new errors; repeat for `app`. +- `mdl-examples/doctype-tests/`: one script that creates a module with settings, + alters both keys and describes it. + +## Open Questions + +1. **Solution modules.** `ProtectedModuleType: Solution` exists in the + metamodel, but the dialog in TestApp offers only App and Add-on. It probably + appears only in a solution project. Out of scope until one is measured. +2. **What else Location must carry** before it becomes writable (*Later: + Location*). + +### Resolved + +- **A new module's defaults in 11.16** are *App module* and `1.0.0`, which is + what `CreateModule` already writes and what `describe` omits. +- **Switching App ↔ Add-on** changes only `ExportLevel`. +- **Location becomes writable**, in a later step after its measurement. diff --git a/docs/11-proposals/show-describe-module-settings.md b/docs/11-proposals/show-describe-module-settings.md index e16fd4fb49..b9b77ab8e9 100644 --- a/docs/11-proposals/show-describe-module-settings.md +++ b/docs/11-proposals/show-describe-module-settings.md @@ -1,10 +1,12 @@ --- title: SHOW/DESCRIBE Module Settings -status: proposed +status: superseded --- # Proposal: SHOW/DESCRIBE Module Settings +> **Status: SUPERSEDED** by [`PROPOSAL_module_settings.md`](./PROPOSAL_module_settings.md) (ako/mxcli#1921), which measures the storage on Mendix 11.16 and adds the write side. This proposal is retained for historical context only; its BSON notes are not reliable (`version` is stored as `Version`, and `ProtectedModuleType` is never `none`). + ## Overview **Document type:** `Projects$ModuleSettings` diff --git a/mdl-examples/doctype-tests/30-module-settings.mdl b/mdl-examples/doctype-tests/30-module-settings.mdl new file mode 100644 index 0000000000..057e5890c2 --- /dev/null +++ b/mdl-examples/doctype-tests/30-module-settings.mdl @@ -0,0 +1,39 @@ +mdl 1; +-- An add-on module writes Projects$ModuleSettings.ProtectedModuleType, which +-- exists from Mendix 9.12; mxcli refuses `ModuleType: addon` below it. +-- @version: 9.12+ +-- ============================================================================ +-- Module Settings Examples (ako/mxcli#1921) +-- ============================================================================ +-- +-- The General tab of Studio Pro's Module settings dialog: the module version +-- and the module type. Stored in Projects$ModuleSettings as Version and +-- ExportLevel (Source = App module, Protected = Add-on module). +-- +-- Features demonstrated: +-- - CREATE OR MODIFY MODULE ... ( Version: ..., ModuleType: ... ) +-- - ALTER MODULE ... SET ( ... ) +-- - DESCRIBE MODULE (prints the settings that differ from App / 1.0.0) +-- - LIST MODULES (Version and Type columns) +-- +-- ============================================================================ + +-- A new module with its settings +create or modify module SettingsTest ( + Version: '2.3.1', + ModuleType: addon, +); + +describe module SettingsTest; + +-- Change one setting; the other keeps its value +alter module SettingsTest set (Version: '2.4.0'); + +-- Back to an app module +alter module SettingsTest set (ModuleType: app); + +-- Restating the stored settings writes nothing +create or modify module SettingsTest (Version: '2.4.0'); + +describe module SettingsTest; +list modules; diff --git a/mdl/ast/ast_enumeration.go b/mdl/ast/ast_enumeration.go index bb08afe61c..efef113b0d 100644 --- a/mdl/ast/ast_enumeration.go +++ b/mdl/ast/ast_enumeration.go @@ -26,6 +26,8 @@ type CreateModuleStmt struct { // DomainModels$DomainModel does (mendixlabs/mxcli#1314). Documentation string DocumentationSet bool // see mendixlabs/mxcli#1018: absent preserves, empty clears + // Settings is the `( Version: …, ModuleType: … )` list (ako/mxcli#1921). + Settings ModuleSettingsClause } func (s *CreateModuleStmt) isStatement() {} diff --git a/mdl/ast/ast_module_settings.go b/mdl/ast/ast_module_settings.go index b6363f8348..f4b098950d 100644 --- a/mdl/ast/ast_module_settings.go +++ b/mdl/ast/ast_module_settings.go @@ -10,6 +10,27 @@ type AlterModuleJarDepStmt struct { func (s *AlterModuleJarDepStmt) isStatement() {} +// ModuleSettingsClause is the General tab of Studio Pro's Module settings +// dialog as MDL states it (ako/mxcli#1921). A nil field is a setting the +// statement does not name, which keeps its stored value. +type ModuleSettingsClause struct { + Version *string // "2.3.1"; SemVer, checked by the visitor + ModuleType *string // "app" | "addon" +} + +// IsEmpty reports whether the clause names no setting. +func (c ModuleSettingsClause) IsEmpty() bool { + return c.Version == nil && c.ModuleType == nil +} + +// AlterModuleSetStmt represents: ALTER MODULE name SET ( Key: value, … ) +type AlterModuleSetStmt struct { + ModuleName string + Settings ModuleSettingsClause +} + +func (s *AlterModuleSetStmt) isStatement() {} + // JarDepAction is the common interface for JAR dependency sub-actions. type JarDepAction interface { isJarDepAction() diff --git a/mdl/backend/modelsdk/microflow_external_action_assoc_test.go b/mdl/backend/modelsdk/microflow_external_action_assoc_test.go new file mode 100644 index 0000000000..66f2a6cc0a --- /dev/null +++ b/mdl/backend/modelsdk/microflow_external_action_assoc_test.go @@ -0,0 +1,95 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "os" + "reflect" + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/modelsdk/codec" + "github.com/mendixlabs/mxcli/modelsdk/element" + genMf "github.com/mendixlabs/mxcli/modelsdk/gen/microflows" +) + +// ako/TestApp's Odata.IncludeAssociationsMF and its control _Off (Studio Pro +// 11.16.0): the same CreateBuffer call on the Bug1914 service, with +// "Associations to include" ticked for Quantity and the Unit under it, and with +// nothing ticked. +const ( + includeAssocFixture = "testdata/TestApp.IncludeAssociationsMF.11_16.bson" + includeAssocOffFixture = "testdata/TestApp.IncludeAssociationsMF_Off.11_16.bson" +) + +// storedExternalAction returns the fixture's one CallExternalAction, as the +// decoded element and as its stored bytes. +func storedExternalAction(t *testing.T, path string) (element.Element, bson.Raw) { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + root, err := codec.NewDecoder(codec.DefaultRegistry).Decode(bson.Raw(data)) + if err != nil { + t.Fatal(err) + } + var found element.Element + element.Walk(root, func(e element.Element) bool { + if _, ok := e.(*genMf.CallExternalAction); ok { + found = e + } + return found == nil + }) + if found == nil { + t.Fatalf("%s: no CallExternalAction", path) + } + return found, found.Raw() +} + +// includedAssociationsByParameter reads each parameter mapping's +// IncludedAssociations list as plain values, $IDs included. An empty list — +// Studio Pro writes the bare marker on every mapping — reads as absent: the +// writer leaves empty lists out (#1073's finding), and that is not this test's +// question. +func includedAssociationsByParameter(t *testing.T, call bson.Raw) map[string]any { + t.Helper() + out := map[string]any{} + for _, md := range rawDocElements(call, "ParameterMappings") { + var m bson.M + if err := bson.Unmarshal(md, &m); err != nil { + t.Fatal(err) + } + var v any + if list, ok := m["IncludedAssociations"].(bson.A); ok && len(list) > 1 { + v = list + } + out[rawStr(md, "ParameterName")] = v + } + return out +} + +// #1914: "Associations to include" is a tree of Microflows$IncludedAssociation +// on each parameter mapping, and the reader and writer both ignored it, so any +// rewrite of the microflow — CREATE OR MODIFY, or an ALTER that re-encodes it — +// wrote the call back without it, and the OData request silently lost the +// associated objects at runtime. +func TestCallExternalAction_IncludedAssociationsRoundTrip(t *testing.T) { + for _, fixture := range []string{includeAssocFixture, includeAssocOffFixture} { + t.Run(fixture, func(t *testing.T) { + el, stored := storedExternalAction(t, fixture) + out, err := (&codec.Encoder{}).Encode(microflowActionToGen(actionFromGen(el))) + if err != nil { + t.Fatalf("encode: %v", err) + } + want := includedAssociationsByParameter(t, stored) + got := includedAssociationsByParameter(t, bson.Raw(out)) + for param, w := range want { + if !reflect.DeepEqual(got[param], w) { + t.Errorf("parameter %s: IncludedAssociations written as\n %v\nstored as\n %v", param, got[param], w) + } + } + }) + } +} diff --git a/mdl/backend/modelsdk/microflow_external_action_write.go b/mdl/backend/modelsdk/microflow_external_action_write.go index 30fdec21a8..9f7d631daa 100644 --- a/mdl/backend/modelsdk/microflow_external_action_write.go +++ b/mdl/backend/modelsdk/microflow_external_action_write.go @@ -16,6 +16,10 @@ func init() { codec.RegisterTypeDefaults("Microflows$CallExternalAction", codec.TypeDefaults{ MandatoryListMarkers: map[string]int32{"ParameterMappings": 3}, }) + // Associations to include are marker-2 lists at both levels, measured on + // ako/TestApp's Odata.IncludeAssociationsMF (Studio Pro 11.16.0). + codec.RegisterPropertyListMarker("Microflows$ExternalActionParameterMapping", "IncludedAssociations", 2) + codec.RegisterPropertyListMarker("Microflows$IncludedAssociation", "IncludedAssociations", 2) } // callExternalActionToGen builds a Microflows$CallExternalAction @@ -44,6 +48,11 @@ func callExternalActionToGen(a *microflows.CallExternalAction) element.Element { if pm.ParameterDataType != "" { addPart(m, "ParameterType", externalActionReturnTypeToGen(pm.ParameterDataType, pm.ParameterEntity)) } + // Written only when there is something to include: the empty list is + // version-dependent (Mendix 10.11+), and leaving it out builds clean. + if len(pm.IncludedAssociations) > 0 { + addPartList(m, "IncludedAssociations", includedAssociationsToGen(pm.IncludedAssociations)) + } mappings = append(mappings, m) } if len(mappings) > 0 { @@ -52,6 +61,26 @@ func callExternalActionToGen(a *microflows.CallExternalAction) element.Element { return g } +// includedAssociationsToGen writes a parameter's Associations to include in +// Studio Pro's shape: each association with IsParent, the ones ticked under it +// nested in its own IncludedAssociations (#1914). +func includedAssociationsToGen(in []*microflows.IncludedAssociation) []element.Element { + out := make([]element.Element, 0, len(in)) + for _, ia := range in { + e := newElem("Microflows$IncludedAssociation", string(ia.ID)) + addStr(e, "Association", ia.Association) + // Studio Pro writes the nested list, empty or not, before IsParent. + if len(ia.IncludedAssociations) > 0 { + addPartList(e, "IncludedAssociations", includedAssociationsToGen(ia.IncludedAssociations)) + } else { + addEmptyTypedList(e, "IncludedAssociations", 2) + } + addBool(e, "IsParent", ia.IsParent) + out = append(out, e) + } + return out +} + // externalActionReturnTypeToGen maps a Mendix kind name to the DataTypes$ element // stored in CallExternalAction.VariableDataType. Mirrors // sdk/mpr.serializeExternalActionReturnType. diff --git a/mdl/backend/modelsdk/microflow_read_actions.go b/mdl/backend/modelsdk/microflow_read_actions.go index ae365b52e3..ebf67374ca 100644 --- a/mdl/backend/modelsdk/microflow_read_actions.go +++ b/mdl/backend/modelsdk/microflow_read_actions.go @@ -616,7 +616,8 @@ func actionFromGen(el element.Element) microflows.MicroflowAction { ResultVariableName: rawStr(raw, "VariableName"), } out.UseReturnVariable = out.ResultVariableName != "" - for _, md := range rawDocElements(raw, "ParameterMappings") { + genMappings := a.ParameterMappingsItems() + for i, md := range rawDocElements(raw, "ParameterMappings") { pm := µflows.ExternalActionParameterMapping{ ParameterName: rawStr(md, "ParameterName"), Argument: rawStr(md, "Argument"), @@ -624,6 +625,16 @@ func actionFromGen(el element.Element) microflows.MicroflowAction { if b, ok := md.Lookup("CanBeEmpty").BooleanOK(); ok { pm.CanBeEmpty = b } + // The parameter's entity, so a rewrite can tell whether the stored + // IncludedAssociations still belong to the argument it binds. + if pt, ok := md.Lookup("ParameterType").DocumentOK(); ok { + pm.ParameterEntity = rawStr(pt, "Entity") + } + if i < len(genMappings) { + if gm, ok := genMappings[i].(*genMf.ExternalActionParameterMapping); ok { + pm.IncludedAssociations = includedAssociationsFromGen(gm.IncludedAssociationsItems()) + } + } out.ParameterMappings = append(out.ParameterMappings, pm) } out.ID = model.ID(a.ID()) @@ -1552,6 +1563,27 @@ func readWebServiceRequestBody(raw bson.Raw, out *microflows.WebServiceCallActio } } +// includedAssociationsFromGen reads a "call external action" parameter's +// Associations to include, nested ones under the association they hang from, +// keeping each element's $ID so a rewrite carries them unchanged (#1914). +func includedAssociationsFromGen(items []element.Element) []*microflows.IncludedAssociation { + var out []*microflows.IncludedAssociation + for _, it := range items { + ia, ok := it.(*genMf.IncludedAssociation) + if !ok { + continue + } + a := µflows.IncludedAssociation{ + Association: ia.AssociationQualifiedName(), + IsParent: ia.IsParent(), + IncludedAssociations: includedAssociationsFromGen(ia.IncludedAssociationsItems()), + } + a.ID = model.ID(ia.ID()) + out = append(out, a) + } + return out +} + func rawDocElements(raw bson.Raw, key string) []bson.Raw { arr, ok := raw.Lookup(key).ArrayOK() if !ok { diff --git a/mdl/backend/modelsdk/module_settings_carry_test.go b/mdl/backend/modelsdk/module_settings_carry_test.go new file mode 100644 index 0000000000..32799c20c9 --- /dev/null +++ b/mdl/backend/modelsdk/module_settings_carry_test.go @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "os" + "reflect" + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" +) + +// ako/TestApp's WorkflowCommons settings (Marketplace 4.11.0, Studio Pro +// 11.16.0): a Checksum, a PackageId and EnableDetailedTroubleshooting false — +// three values that differ from their defaults, and that mxcli does not model. +const workflowCommonsSettingsFixture = "testdata/TestApp.WorkflowCommons.ModuleSettings.11_16.bson" + +// #1920: a jar dependency change rebuilt the settings unit from the keys mxcli +// models, so the write path filled the rest with defaults: the PackageId and +// the Marketplace Checksum were wiped and EnableDetailedTroubleshooting turned +// true. Every key but JarDependencies must come back as Studio Pro stored it. +func TestModuleSettingsContents_KeepsWhatItDoesNotModel(t *testing.T) { + stored, err := os.ReadFile(workflowCommonsSettingsFixture) + if err != nil { + t.Fatal(err) + } + var before bson.M + if err := bson.Unmarshal(stored, &before); err != nil { + t.Fatal(err) + } + // The controls: the fixture must hold non-default values, or a rebuild + // would pass this test too. + if before["Checksum"] == "" || before["PackageId"] == "" || before["EnableDetailedTroubleshooting"] != false { + t.Fatalf("fixture lost its non-default values: %v", before) + } + + ms := &types.ModuleSettings{ + ID: model.ID("7a01f032-a89d-46c1-ac0a-9cf136570db5"), + ExportLevel: "Source", + ProtectedModuleType: "AddOn", + Version: "4.11.0", + JarDependencies: []*types.JarDependency{{ + GroupID: "org.example", ArtifactID: "probe", Version: "1.0.0", IsIncluded: true, + }}, + } + out, err := moduleSettingsContents(stored, ms) + if err != nil { + t.Fatal(err) + } + var after bson.M + if err := bson.Unmarshal(out, &after); err != nil { + t.Fatal(err) + } + for k, v := range before { + if k == "JarDependencies" { + continue + } + if !reflect.DeepEqual(after[k], v) { + t.Errorf("%s: stored %v, written %v", k, v, after[k]) + } + } + if deps, _ := after["JarDependencies"].(bson.A); len(deps) != 2 { + t.Errorf("JarDependencies = %v, want the marker and the one dependency", after["JarDependencies"]) + } + if raw := bson.Raw(out); len(raw.Index(0).Key()) == 0 || raw.Index(0).Key() != "$ID" { + t.Errorf("first key = %q, want $ID", raw.Index(0).Key()) + } +} + +// A statement that does change a modelled key still changes it. +func TestModuleSettingsContents_WritesModelledKeys(t *testing.T) { + stored, err := os.ReadFile(workflowCommonsSettingsFixture) + if err != nil { + t.Fatal(err) + } + ms := &types.ModuleSettings{ID: "7a01f032-a89d-46c1-ac0a-9cf136570db5", Version: "4.12.0"} + out, err := moduleSettingsContents(stored, ms) + if err != nil { + t.Fatal(err) + } + var after bson.M + if err := bson.Unmarshal(out, &after); err != nil { + t.Fatal(err) + } + if after["Version"] != "4.12.0" { + t.Errorf("Version = %v, want 4.12.0", after["Version"]) + } +} diff --git a/mdl/backend/modelsdk/module_settings_write.go b/mdl/backend/modelsdk/module_settings_write.go index 30eb3eb3d7..886c951346 100644 --- a/mdl/backend/modelsdk/module_settings_write.go +++ b/mdl/backend/modelsdk/module_settings_write.go @@ -29,8 +29,9 @@ func (b *Backend) GetModuleSettings(moduleID model.ID) (*types.ModuleSettings, e } // UpdateModuleSettings rewrites the Projects$ModuleSettings unit in place, -// including all JarDependencies and their Exclusions. Mirrors the legacy -// serializer (pure BSON; JarDependencies/Exclusions use the marker-2 array form). +// including all JarDependencies and their Exclusions (marker-2 arrays). It +// writes onto the stored unit, so the keys mxcli does not model keep their +// values (#1920). func (b *Backend) UpdateModuleSettings(ms *types.ModuleSettings) error { if ms == nil { return fmt.Errorf("UpdateModuleSettings: nil module settings") @@ -38,7 +39,27 @@ func (b *Backend) UpdateModuleSettings(ms *types.ModuleSettings) error { if b.writer == nil { return fmt.Errorf("UpdateModuleSettings: not connected for writing") } + stored, err := b.reader.GetRawUnitBytes(string(ms.ID)) + if err != nil { + return fmt.Errorf("UpdateModuleSettings: read stored settings: %w", err) + } + contents, err := moduleSettingsContents(stored, ms) + if err != nil { + return err + } + return b.writer.UpdateRawUnit(string(ms.ID), contents) +} +// moduleSettingsContents is the stored settings unit with the keys ms models +// written into it. +// +// It starts from the stored keys, not from an empty document: the unit also +// holds PackageId (Studio Pro's Module ID), a Marketplace module's Checksum, +// EnableDetailedTroubleshooting and more that mxcli does not model. A rebuild +// from the modelled keys alone had the write path fill those with defaults, so +// a jar dependency change wiped the Module ID and the Checksum and switched +// detailed troubleshooting on (#1920). +func moduleSettingsContents(stored []byte, ms *types.ModuleSettings) ([]byte, error) { deps := bson.A{int32(2)} // listType marker for _, d := range ms.JarDependencies { depDoc := bson.M{ @@ -64,7 +85,13 @@ func (b *Backend) UpdateModuleSettings(ms *types.ModuleSettings) error { deps = append(deps, depDoc) } - doc := bson.M{ + doc := bson.M{} + if len(stored) > 0 { + if err := bson.Unmarshal(stored, &doc); err != nil { + return nil, fmt.Errorf("UpdateModuleSettings: decode stored settings: %w", err) + } + } + for k, v := range map[string]any{ "$ID": bsonutil.IDToBsonBinary(string(ms.ID)), "$Type": "Projects$ModuleSettings", "BasedOnVersion": ms.BasedOnVersion, @@ -74,14 +101,16 @@ func (b *Backend) UpdateModuleSettings(ms *types.ModuleSettings) error { "ProtectedModuleType": orDefault(ms.ProtectedModuleType, "AddOn"), "SolutionIdentifier": ms.SolutionIdentifier, "Version": orDefault(ms.Version, "1.0.0"), + } { + doc[k] = v } // Mendix 11.12+ requires "$ID" first in every storage object; doc and its // nested JarDependency/Exclusion maps are unordered, so order on write. contents, err := bson.Marshal(bsonutil.OrderStorageValue(doc)) if err != nil { - return fmt.Errorf("UpdateModuleSettings: marshal: %w", err) + return nil, fmt.Errorf("UpdateModuleSettings: marshal: %w", err) } - return b.writer.UpdateRawUnit(string(ms.ID), contents) + return contents, nil } // moduleSettingsID returns a binary-UUID $ID for a jar-dependency / exclusion, diff --git a/mdl/backend/modelsdk/testdata/TestApp.IncludeAssociationsMF.11_16.bson b/mdl/backend/modelsdk/testdata/TestApp.IncludeAssociationsMF.11_16.bson new file mode 100644 index 0000000000..e69ffc9457 Binary files /dev/null and b/mdl/backend/modelsdk/testdata/TestApp.IncludeAssociationsMF.11_16.bson differ diff --git a/mdl/backend/modelsdk/testdata/TestApp.IncludeAssociationsMF_Off.11_16.bson b/mdl/backend/modelsdk/testdata/TestApp.IncludeAssociationsMF_Off.11_16.bson new file mode 100644 index 0000000000..f026f4fcd2 Binary files /dev/null and b/mdl/backend/modelsdk/testdata/TestApp.IncludeAssociationsMF_Off.11_16.bson differ diff --git a/mdl/backend/modelsdk/testdata/TestApp.WorkflowCommons.ModuleSettings.11_16.bson b/mdl/backend/modelsdk/testdata/TestApp.WorkflowCommons.ModuleSettings.11_16.bson new file mode 100644 index 0000000000..578d79c9ca Binary files /dev/null and b/mdl/backend/modelsdk/testdata/TestApp.WorkflowCommons.ModuleSettings.11_16.bson differ diff --git a/mdl/executor/cmd_alter_flow.go b/mdl/executor/cmd_alter_flow.go index 26cc285c73..0ff080a245 100644 --- a/mdl/executor/cmd_alter_flow.go +++ b/mdl/executor/cmd_alter_flow.go @@ -771,6 +771,13 @@ func (a *alterFlowContext) splitCondition(ctx *ExecContext, op *ast.AlterFlowOpe if missing := a.unscopedRefs(expr, target.ID); len(missing) > 0 { return "", "", fmt.Errorf("the condition uses %s, which is not declared on the path to the decision", strings.Join(missing, ", ")) } + // The `if` states the condition as describe printed it. For a decision + // whose true path goes straight on, describe prints the negated condition + // with the paths swapped; the stored flows do not move, so storing the + // printed condition as it is would invert the decision (ako/mxcli#1930). + if decisionPrintedNegated(ctx, a.mf.ObjectCollection, target.ID) { + expr = negateExpression(expr) + } caption = split.Caption if caption == old.Expression { caption = expr diff --git a/mdl/executor/cmd_microflows_build.go b/mdl/executor/cmd_microflows_build.go index 8e903a5916..e79a7989eb 100644 --- a/mdl/executor/cmd_microflows_build.go +++ b/mdl/executor/cmd_microflows_build.go @@ -174,6 +174,8 @@ func buildMicroflowFromStmt(ctx *ExecContext, s *ast.CreateMicroflowStmt, opts b var existingDocumentation string preserveDocumentation := false var existingActionInfo, existingWorkflowInfo *types.MicroflowActionInfo + // The stored flow, for what its activities hold that MDL cannot restate. + var existingObjects *microflows.MicroflowObjectCollection existingMicroflows, err := microflowsNamed(ctx, s.Name.Name) if err != nil { return nil, mdlerrors.NewBackend("check existing microflows", err) @@ -211,6 +213,7 @@ func buildMicroflowFromStmt(ctx *ExecContext, s *ast.CreateMicroflowStmt, opts b existingDocumentation = existing.Documentation preserveDocumentation = true existingWorkflowInfo = existing.WorkflowActionInfo + existingObjects = existing.ObjectCollection } // For CREATE OR REPLACE/MODIFY, reuse the existing ID to preserve references @@ -464,6 +467,9 @@ func buildMicroflowFromStmt(ctx *ExecContext, s *ast.CreateMicroflowStmt, opts b } return nil, fmt.Errorf("%s", errMsg.String()) } + if err := carryIncludedAssociations(mf.ObjectCollection, existingObjects, qualifiedName); err != nil { + return nil, err + } return &builtFlow{ Microflow: mf, ContainerID: containerID, diff --git a/mdl/executor/cmd_microflows_format_action_test.go b/mdl/executor/cmd_microflows_format_action_test.go index eb510f46bb..830525fd74 100644 --- a/mdl/executor/cmd_microflows_format_action_test.go +++ b/mdl/executor/cmd_microflows_format_action_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/backend/mock" mdltypes "github.com/mendixlabs/mxcli/mdl/types" "github.com/mendixlabs/mxcli/mdl/visitor" @@ -688,6 +689,35 @@ func TestFormatAction_ValidationFeedback(t *testing.T) { } } +// What describe writes for a validation feedback on an attribute named Error +// must parse back: `$E/Error message` used to lex as the `error message` clause +// keyword, so describe output for such a microflow failed `check` with "missing +// MESSAGE" (mendixlabs/mxcli#1720). +func TestFormatAction_ValidationFeedback_MemberNamedErrorReparses(t *testing.T) { + e := newTestExecutor() + for _, attr := range []string{"Error", "error", "Message", "ErrorMessage"} { + action := µflows.ValidationFeedbackAction{ + ObjectVariable: "Entity2", + AttributeName: "ExamplesModule.Entity2." + attr, + Template: &model.Text{ + Translations: map[string]string{"en_US": "not good"}, + }, + } + line := e.formatAction(action, nil, nil) + src := "create or modify microflow ExamplesModule.ACT ($Entity2: ExamplesModule.Entity2)\nbegin\n " + + line + "\n return;\nend;" + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("%s: describe output does not parse: %v\n%s", attr, errs, src) + } + vf := prog.Statements[0].(*ast.CreateMicroflowStmt).Body[0].(*ast.ValidationFeedbackStmt) + segs := vf.AttributePath.Segments + if len(segs) != 1 || segs[0].Name != attr { + t.Errorf("%s: read back as %v", attr, segs) + } + } +} + func TestFormatAction_ValidationFeedback_ObjectOnlyTarget(t *testing.T) { e := newTestExecutor() action := µflows.ValidationFeedbackAction{ diff --git a/mdl/executor/cmd_microflows_show_helpers.go b/mdl/executor/cmd_microflows_show_helpers.go index 7a116d6d0f..c584f9b8a4 100644 --- a/mdl/executor/cmd_microflows_show_helpers.go +++ b/mdl/executor/cmd_microflows_show_helpers.go @@ -3123,31 +3123,71 @@ func negateIfCondition(stmt string) string { const prefix = "if " const suffix = " then" if strings.HasPrefix(stmt, prefix) && strings.HasSuffix(stmt, suffix) { - cond := stmt[len(prefix) : len(stmt)-len(suffix)] - // Avoid double-negation: not(not(x)) → x - // Only unwrap if the outer parens are balanced (depth returns to 0 at the final char) - if strings.HasPrefix(cond, "not(") && strings.HasSuffix(cond, ")") { - inner := cond[4 : len(cond)-1] - depth := 0 - balanced := true - for _, ch := range inner { - if ch == '(' { - depth++ - } else if ch == ')' { - depth-- - if depth < 0 { - balanced = false - break - } + return prefix + negateExpression(stmt[len(prefix):len(stmt)-len(suffix)]) + suffix + } + return stmt +} + +// negateExpression is cond's negation as describe spells it: not(x) becomes +// x, anything else not(cond). It is its own inverse, which is what lets a +// condition edit undo the empty-then swap (decisionPrintedNegated). +func negateExpression(cond string) string { + // Avoid double-negation: not(not(x)) → x + // Only unwrap if the outer parens are balanced (depth returns to 0 at the final char) + if strings.HasPrefix(cond, "not(") && strings.HasSuffix(cond, ")") { + inner := cond[4 : len(cond)-1] + depth := 0 + balanced := true + for _, ch := range inner { + if ch == '(' { + depth++ + } else if ch == ')' { + depth-- + if depth < 0 { + balanced = false + break } } - if balanced && depth == 0 { - return prefix + inner + suffix + } + if balanced && depth == 0 { + return inner + } + } + return "not(" + cond + ")" +} + +// decisionPrintedNegated reports whether describe prints the decision splitID +// with the empty-then swap: its condition negated and its paths swapped, because +// its true path goes straight to its merge and its false path does not. The +// condition an `if` states for such a decision is the negation of the one +// stored, so whatever replaces it must be negated back (ako/mxcli#1930). It +// looks in the loop bodies too, each printed from its own collection. +func decisionPrintedNegated(ctx *ExecContext, oc *microflows.MicroflowObjectCollection, splitID model.ID) bool { + if oc == nil { + return false + } + activityMap := make(map[model.ID]microflows.MicroflowObject, len(oc.Objects)) + for _, obj := range oc.Objects { + activityMap[obj.GetID()] = obj + } + if _, here := activityMap[splitID]; !here { + for _, obj := range oc.Objects { + if loop, ok := obj.(*microflows.LoopedActivity); ok && decisionPrintedNegated(ctx, loop.ObjectCollection, splitID) { + return true } } - return prefix + "not(" + cond + ")" + suffix + return false } - return stmt + var flows []*microflows.SequenceFlow + for _, f := range oc.Flows { + if f.OriginID == splitID { + flows = append(flows, f) + } + } + mergeID := findSplitMergePoints(ctx, oc, activityMap)[splitID] + trueFlow, falseFlow := findBranchFlows(flows) + return trueFlow != nil && falseFlow != nil && mergeID != "" && + trueFlow.DestinationID == mergeID && falseFlow.DestinationID != mergeID } func (e *Executor) collectErrorHandlerStatements( diff --git a/mdl/executor/cmd_module_settings.go b/mdl/executor/cmd_module_settings.go new file mode 100644 index 0000000000..a6f4d30bdd --- /dev/null +++ b/mdl/executor/cmd_module_settings.go @@ -0,0 +1,116 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" +) + +// The General tab of Studio Pro's Module settings dialog (ako/mxcli#1921), +// measured on 11.16: +// +// Module type ExportLevel: Source = App module, Protected = Add-on module. +// ProtectedModuleType is AddOn on every module, app modules +// included, so it alone does not say "add-on". +// Module version Version, a SemVer string. +// Location Projects$ModuleImpl.FromAppStore — read-only here. +// +// A new module is App, 1.0.0: those are the defaults describe leaves out. +const ( + defaultModuleVersion = "1.0.0" + moduleTypeApp = "app" + moduleTypeAddOn = "addon" +) + +// moduleTypeOf is the MDL ModuleType of stored settings, or "" for one MDL +// cannot name (a solution module). +func moduleTypeOf(ms *types.ModuleSettings) string { + if ms == nil || ms.ExportLevel != "Protected" { + return moduleTypeApp + } + if ms.ProtectedModuleType == "" || ms.ProtectedModuleType == "AddOn" { + return moduleTypeAddOn + } + return "" +} + +// applyModuleSettings writes what the clause states onto the stored settings. +// It reports whether anything changed. +func applyModuleSettings(ms *types.ModuleSettings, c ast.ModuleSettingsClause) bool { + changed := false + if c.Version != nil && ms.Version != *c.Version { + ms.Version = *c.Version + changed = true + } + if c.ModuleType != nil && moduleTypeOf(ms) != *c.ModuleType { + switch *c.ModuleType { + case moduleTypeAddOn: + ms.ExportLevel, ms.ProtectedModuleType = "Protected", "AddOn" + case moduleTypeApp: + ms.ExportLevel = "Source" + } + changed = true + } + return changed +} + +// setModuleSettings applies a settings clause to an existing module. +func setModuleSettings(ctx *ExecContext, module *model.Module, c ast.ModuleSettingsClause) error { + if c.IsEmpty() { + return nil + } + ms, err := ctx.Backend.GetModuleSettings(module.ID) + if err != nil { + return mdlerrors.NewBackend(fmt.Sprintf("read settings of module %s", module.Name), err) + } + if !applyModuleSettings(ms, c) { + fmt.Fprintf(ctx.Output, "Module settings of %s unchanged\n", module.Name) + return nil + } + if err := ctx.Backend.UpdateModuleSettings(ms); err != nil { + return mdlerrors.NewBackend(fmt.Sprintf("update settings of module %s", module.Name), err) + } + fmt.Fprintf(ctx.Output, "Updated module settings: %s\n", module.Name) + return nil +} + +// execAlterModuleSet handles ALTER MODULE name SET ( Key: value, … ). +func execAlterModuleSet(ctx *ExecContext, s *ast.AlterModuleSetStmt) error { + if !ctx.Connected() { + return mdlerrors.NewNotConnected() + } + module, err := ctx.Backend.GetModuleByName(s.ModuleName) + if err != nil { + return mdlerrors.NewBackend("find module "+s.ModuleName, err) + } + if module == nil { + return mdlerrors.NewNotFound("module", s.ModuleName) + } + return setModuleSettings(ctx, module, s.Settings) +} + +// moduleSettingsMDL is the `( … )` list describe prints after the module name: +// only the settings that differ from a new module's, so an untouched module +// describes as it always did. "" when there are none. +func moduleSettingsMDL(ctx *ExecContext, ms *types.ModuleSettings) string { + if ms == nil { + return "" + } + var props []string + if ms.Version != "" && ms.Version != defaultModuleVersion { + props = append(props, fmt.Sprintf("Version: %s", mdlQuote(ctx, ms.Version))) + } + if moduleTypeOf(ms) == moduleTypeAddOn { + props = append(props, "ModuleType: "+moduleTypeAddOn) + } + if len(props) == 0 { + return "" + } + return " (\n " + strings.Join(props, ",\n ") + ",\n)" +} diff --git a/mdl/executor/cmd_module_settings_test.go b/mdl/executor/cmd_module_settings_test.go new file mode 100644 index 0000000000..c47194d579 --- /dev/null +++ b/mdl/executor/cmd_module_settings_test.go @@ -0,0 +1,114 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" +) + +// Studio Pro's stored settings, measured on ako/TestApp (11.16): an app module +// as every untouched one is, and the probe set to Add-on 2.3.1. Every module +// stores ProtectedModuleType AddOn, so it is in the app module too. +func appModuleSettings() *types.ModuleSettings { + return &types.ModuleSettings{ID: "ms-app", ContainerID: "mod-1", ExportLevel: "Source", ProtectedModuleType: "AddOn", Version: "1.0.0"} +} + +func addOnModuleSettings() *types.ModuleSettings { + return &types.ModuleSettings{ID: "ms-addon", ContainerID: "mod-1", ExportLevel: "Protected", ProtectedModuleType: "AddOn", Version: "2.3.1"} +} + +// ako/mxcli#1921: the module type is ExportLevel, not ProtectedModuleType. +// Reading the latter would make every app module an add-on. +func TestModuleTypeOf(t *testing.T) { + for _, c := range []struct { + ms *types.ModuleSettings + want string + }{ + {appModuleSettings(), "app"}, + {addOnModuleSettings(), "addon"}, + {&types.ModuleSettings{ExportLevel: "Protected", ProtectedModuleType: "Solution"}, ""}, + } { + if got := moduleTypeOf(c.ms); got != c.want { + t.Errorf("%+v: type %q, want %q", c.ms, got, c.want) + } + } +} + +// describe prints only what differs from a new module (App, 1.0.0), and what it +// prints parses back to the same settings. +func TestModuleSettingsMDL(t *testing.T) { + ctx, _ := newMockCtx(t) + if got := moduleSettingsMDL(ctx, appModuleSettings()); got != "" { + t.Errorf("an untouched app module prints settings: %q", got) + } + out := "create or modify module Probe" + moduleSettingsMDL(ctx, addOnModuleSettings()) + ";" + prog := parseMDL(t, out) + s, ok := prog.Statements[0].(*ast.CreateModuleStmt) + if !ok || s.Settings.Version == nil || *s.Settings.Version != "2.3.1" || + s.Settings.ModuleType == nil || *s.Settings.ModuleType != "addon" { + t.Errorf("describe output does not read back as the stored settings:\n%s\n%+v", out, prog.Statements[0]) + } + // A solution module has no MDL spelling: no ModuleType, so a rerun keeps it. + sol := &types.ModuleSettings{ExportLevel: "Protected", ProtectedModuleType: "Solution", Version: "1.0.0"} + if got := moduleSettingsMDL(ctx, sol); got != "" { + t.Errorf("a solution module prints %q", got) + } +} + +// alter module … set writes exactly what it names, onto the stored settings; +// a statement that restates them writes nothing. +func TestAlterModuleSet(t *testing.T) { + str := func(s string) *string { return &s } + run := func(t *testing.T, stored *types.ModuleSettings, c ast.ModuleSettingsClause) (*types.ModuleSettings, string) { + t.Helper() + var written *types.ModuleSettings + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + GetModuleByNameFunc: func(name string) (*model.Module, error) { + return &model.Module{BaseElement: model.BaseElement{ID: "mod-1"}, Name: name}, nil + }, + GetModuleSettingsFunc: func(model.ID) (*types.ModuleSettings, error) { return stored, nil }, + UpdateModuleSettingsFunc: func(ms *types.ModuleSettings) error { + c := *ms + written = &c + return nil + }, + } + ctx, buf := newMockCtx(t, withBackend(mb)) + if err := execAlterModuleSet(ctx, &ast.AlterModuleSetStmt{ModuleName: "Probe", Settings: c}); err != nil { + t.Fatal(err) + } + return written, buf.String() + } + + t.Run("app to add-on", func(t *testing.T) { + w, _ := run(t, appModuleSettings(), ast.ModuleSettingsClause{ModuleType: str("addon")}) + if w == nil || w.ExportLevel != "Protected" || w.ProtectedModuleType != "AddOn" || w.Version != "1.0.0" { + t.Errorf("written %+v, want Protected/AddOn with the version kept", w) + } + }) + t.Run("add-on to app", func(t *testing.T) { + w, _ := run(t, addOnModuleSettings(), ast.ModuleSettingsClause{ModuleType: str("app")}) + if w == nil || w.ExportLevel != "Source" || w.Version != "2.3.1" { + t.Errorf("written %+v, want Source with the version kept", w) + } + }) + t.Run("version only", func(t *testing.T) { + w, _ := run(t, addOnModuleSettings(), ast.ModuleSettingsClause{Version: str("2.4.0")}) + if w == nil || w.Version != "2.4.0" || w.ExportLevel != "Protected" { + t.Errorf("written %+v, want 2.4.0 with the type kept", w) + } + }) + t.Run("control: restated settings write nothing", func(t *testing.T) { + w, out := run(t, addOnModuleSettings(), ast.ModuleSettingsClause{Version: str("2.3.1"), ModuleType: str("addon")}) + if w != nil || !strings.Contains(out, "unchanged") { + t.Errorf("restating the stored settings wrote %+v (output %q)", w, out) + } + }) +} diff --git a/mdl/executor/cmd_modules.go b/mdl/executor/cmd_modules.go index 222ac1efb5..fa1d9823b8 100644 --- a/mdl/executor/cmd_modules.go +++ b/mdl/executor/cmd_modules.go @@ -35,12 +35,20 @@ func execCreateModule(ctx *ExecContext, s *ast.CreateModuleStmt) error { if m.Name == s.Name { // A plain create of an existing module stays the no-op it always // was; `or modify` applies the documentation the statement states. - if s.CreateOrModify && s.DocumentationSet { - return setModuleDocumentation(ctx, m, s.Documentation) + if s.CreateOrModify { + if err := setModuleSettings(ctx, m, s.Settings); err != nil { + return err + } + if s.DocumentationSet { + return setModuleDocumentation(ctx, m, s.Documentation) + } + if !s.Settings.IsEmpty() { + return nil + } } - if s.DocumentationSet { - fmt.Fprintf(ctx.Output, "Module '%s' already exists; documentation unchanged "+ - "(create or modify module sets it)\n", s.Name) + if s.DocumentationSet || !s.Settings.IsEmpty() { + fmt.Fprintf(ctx.Output, "Module '%s' already exists; documentation and settings unchanged "+ + "(create or modify module sets them)\n", s.Name) return nil } fmt.Fprintf(ctx.Output, "Module '%s' already exists\n", s.Name) @@ -61,6 +69,13 @@ func execCreateModule(ctx *ExecContext, s *ast.CreateModuleStmt) error { invalidateModuleCache(ctx) fmt.Fprintf(ctx.Output, "Created module: %s\n", s.Name) + if !s.Settings.IsEmpty() { + // The new module's settings unit holds the defaults; the stated ones + // are written onto it, so its PackageId and the rest are kept. + if err := setModuleSettings(ctx, module, s.Settings); err != nil { + return err + } + } if s.DocumentationSet && s.Documentation != "" { return setModuleDocumentation(ctx, module, s.Documentation) } @@ -533,6 +548,8 @@ func listModules(ctx *ExecContext) error { type row struct { name string source string + version string + moduleType string entities int enums int pages int @@ -550,6 +567,14 @@ func listModules(ctx *ExecContext) error { } var rows []row protections := moduleProtections(ctx) + // Version and Type are the General tab of the Module settings dialog + // (ako/mxcli#1921). + settingsByModule := map[model.ID]*types.ModuleSettings{} + if all, err := ctx.Backend.ListModuleSettings(); err == nil { + for _, ms := range all { + settingsByModule[ms.ContainerID] = ms + } + } for _, m := range modules { // Determine source (AppStore with version or local) @@ -570,9 +595,23 @@ func listModules(ctx *ExecContext) error { source += "Protected " + kind } + version, moduleType := "", "" + if ms := settingsByModule[m.ID]; ms != nil { + version = ms.Version + switch moduleTypeOf(ms) { + case moduleTypeApp: + moduleType = "App" + case moduleTypeAddOn: + moduleType = "Add-on" + default: + moduleType = ms.ProtectedModuleType + } + } r := row{ name: m.Name, source: source, + version: version, + moduleType: moduleType, entities: entityCounts[m.ID], enums: enumCounts[m.ID], pages: pageCounts[m.ID], @@ -593,11 +632,11 @@ func listModules(ctx *ExecContext) error { // Build TableResult result := &TableResult{ - Columns: []string{"Module", "Source", "Entities", "Enums", "Pages", "Snippets", "Microflows", "Nanoflows", "Workflows", "Constants", "JavaActions", "PubREST", "PubOData", "ConOData", "BizEvents", "ExtDB"}, + Columns: []string{"Module", "Source", "Version", "Type", "Entities", "Enums", "Pages", "Snippets", "Microflows", "Nanoflows", "Workflows", "Constants", "JavaActions", "PubREST", "PubOData", "ConOData", "BizEvents", "ExtDB"}, Summary: fmt.Sprintf("(%d modules)", len(modules)), } for _, r := range rows { - result.Rows = append(result.Rows, []any{r.name, r.source, r.entities, r.enums, r.pages, r.snippets, r.microflows, r.nanoflows, r.workflows, r.constants, r.javaActions, r.pubRest, r.pubOData, r.conOData, r.bizEvents, r.extDb}) + result.Rows = append(result.Rows, []any{r.name, r.source, r.version, r.moduleType, r.entities, r.enums, r.pages, r.snippets, r.microflows, r.nanoflows, r.workflows, r.constants, r.javaActions, r.pubRest, r.pubOData, r.conOData, r.bizEvents, r.extDb}) } return writeResult(ctx, result) } @@ -631,11 +670,29 @@ func describeModule(ctx *ExecContext, moduleName string, withAll bool) error { if dm, err := ctx.Backend.GetDomainModel(targetModule.ID); err == nil && dm != nil && dm.Documentation != "" { fmt.Fprintf(ctx.Output, "/**\n * %s\n */\n", strings.ReplaceAll(dm.Documentation, "\n", "\n * ")) } - fmt.Fprintf(ctx.Output, "create or modify module %s;\n", targetModule.Name) + // Listed rather than got by module: the same read moduleProtection makes. + var settings *types.ModuleSettings + if all, err := ctx.Backend.ListModuleSettings(); err == nil { + for _, ms := range all { + if ms.ContainerID == targetModule.ID { + settings = ms + } + } + } + fmt.Fprintf(ctx.Output, "create or modify module %s%s;\n", targetModule.Name, moduleSettingsMDL(ctx, settings)) + // Location has no key yet (PROPOSAL_module_settings.md, "Later: Location"). + if targetModule.FromAppStore { + fmt.Fprintln(ctx.Output, "-- location: Marketplace modules (set in Studio Pro)") + } // A Protected (add-on / solution) module hides every element not marked - // Usable from the rest of the app (CE1406). MDL has no syntax for the - // module's export level, so it prints as a comment (mendixlabs/mxcli#1498). - if kind := moduleProtection(ctx, string(targetModule.ID)); kind != "" { + // Usable from the rest of the app (CE1406), which nothing else printed + // says (mendixlabs/mxcli#1498). An add-on is `ModuleType: addon` above; a + // solution module has no MDL spelling, so it is a comment only. + switch kind := moduleProtection(ctx, string(targetModule.ID)); kind { + case "": + case "AddOn": + fmt.Fprintln(ctx.Output, "-- add-on module: elements not marked Usable cannot be referenced from other modules") + default: fmt.Fprintf(ctx.Output, "-- export level: Protected (%s module) — set in Studio Pro; elements not marked Usable cannot be referenced from other modules\n", kind) } diff --git a/mdl/executor/cmd_modules_export_level_test.go b/mdl/executor/cmd_modules_export_level_test.go index 3978c047d4..727d9190ed 100644 --- a/mdl/executor/cmd_modules_export_level_test.go +++ b/mdl/executor/cmd_modules_export_level_test.go @@ -12,8 +12,8 @@ import ( // mendixlabs/mxcli#1498: nothing mxcli printed said a module was Protected, nor // what an entity's export level was, so the reporter found both by a raw read. -// MDL has no syntax for a module's, so it prints as a comment — the output -// still re-runs, and a rewrite keeps the stored value. Only a Protected +// A module's is `ModuleType: addon` since ako/mxcli#1921, with a comment on +// what it means; the output still re-runs. Only a Protected // module's output changes; Open is the control. func TestDescribe_ProtectedModuleExportLevels(t *testing.T) { ctx := protectedModuleFixture(t, nil) @@ -31,7 +31,9 @@ func TestDescribe_ProtectedModuleExportLevels(t *testing.T) { } out := describe(func() error { return describeModule(ctx, "MyAddOn", false) }) - if !strings.Contains(out, "-- export level: Protected (AddOn module)") { + // An add-on is a setting since ako/mxcli#1921, with the comment saying what + // it means for references. + if !strings.Contains(out, "ModuleType: addon") || !strings.Contains(out, "-- add-on module: elements not marked Usable") { t.Errorf("describe module of a protected module does not say so:\n%s", out) } out = describe(func() error { return describeModule(ctx, "Open", false) }) diff --git a/mdl/executor/external_action_carry.go b/mdl/executor/external_action_carry.go new file mode 100644 index 0000000000..cfeb89e71a --- /dev/null +++ b/mdl/executor/external_action_carry.go @@ -0,0 +1,131 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "sort" + "strings" + + mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// carryIncludedAssociations copies each stored "call external action" +// parameter's Associations to include onto the rewritten call, because MDL has +// no syntax for them: a CREATE OR MODIFY that restated the call wrote it with +// none, and the OData request then silently left out the associated objects +// (#1914). check, mxbuild and the build all pass either way. +// +// A call is matched on its service and action, then each mapping on its +// parameter name. The stored order of a microflow's objects is Studio Pro's, +// not the statement's, so position cannot pair two calls to the same action. +// When those calls hold the same tree it does not matter which gets which; +// when they differ, the rewrite is refused rather than guessed. +// +// A tree is not carried onto a parameter that now binds another entity: its +// associations belong to the entity it was ticked on. +func carryIncludedAssociations(rebuilt, stored *microflows.MicroflowObjectCollection, qualifiedName string) error { + type slot struct { + entity string + tree []*microflows.IncludedAssociation + shape string + } + storedTrees := map[string]*slot{} // service.action|parameter + var conflicts []string + for _, call := range externalActionCalls(stored) { + for _, pm := range call.ParameterMappings { + if len(pm.IncludedAssociations) == 0 { + continue + } + key := externalActionKey(call) + "|" + strings.ToLower(pm.ParameterName) + shape := includedAssociationsShape(pm.IncludedAssociations) + if prev, ok := storedTrees[key]; ok { + if prev.shape != shape || !strings.EqualFold(prev.entity, pm.ParameterEntity) { + conflicts = append(conflicts, call.ConsumedODataService+"."+call.Name+"("+pm.ParameterName+")") + } + continue + } + storedTrees[key] = &slot{entity: pm.ParameterEntity, tree: pm.IncludedAssociations, shape: shape} + } + } + if len(conflicts) > 0 { + sort.Strings(conflicts) + return mdlerrors.NewUnsupported(fmt.Sprintf( + "microflow %s calls %s more than once with different associations to include, "+ + "and MDL cannot say which call has which — rewriting it would mix them up.\n"+ + " Change the microflow in Studio Pro, or give the calls the same associations first.", + qualifiedName, strings.Join(conflicts, ", "))) + } + if len(storedTrees) == 0 { + return nil + } + used := map[string]bool{} + for _, call := range externalActionCalls(rebuilt) { + for _, pm := range call.ParameterMappings { + key := externalActionKey(call) + "|" + strings.ToLower(pm.ParameterName) + s, ok := storedTrees[key] + if !ok || len(pm.IncludedAssociations) > 0 { + continue + } + if s.entity != "" && pm.ParameterEntity != "" && !strings.EqualFold(s.entity, pm.ParameterEntity) { + continue + } + // The first copy keeps the stored $IDs; a second call to the same + // action gets fresh ones, since two elements of a unit cannot share one. + pm.IncludedAssociations = cloneIncludedAssociations(s.tree, !used[key]) + used[key] = true + } + } + return nil +} + +// externalActionCalls returns every CallExternalAction in a flow, loop bodies +// included. +func externalActionCalls(oc *microflows.MicroflowObjectCollection) []*microflows.CallExternalAction { + if oc == nil { + return nil + } + var out []*microflows.CallExternalAction + for _, obj := range oc.Objects { + switch o := obj.(type) { + case *microflows.ActionActivity: + if c, ok := o.Action.(*microflows.CallExternalAction); ok { + out = append(out, c) + } + case *microflows.LoopedActivity: + out = append(out, externalActionCalls(o.ObjectCollection)...) + } + } + return out +} + +func externalActionKey(c *microflows.CallExternalAction) string { + return strings.ToLower(c.ConsumedODataService + "." + c.Name) +} + +// includedAssociationsShape is a tree's content without its $IDs, for telling +// two stored trees apart. +func includedAssociationsShape(in []*microflows.IncludedAssociation) string { + var b strings.Builder + for _, ia := range in { + fmt.Fprintf(&b, "(%s %t %s)", ia.Association, ia.IsParent, includedAssociationsShape(ia.IncludedAssociations)) + } + return b.String() +} + +func cloneIncludedAssociations(in []*microflows.IncludedAssociation, keepIDs bool) []*microflows.IncludedAssociation { + out := make([]*microflows.IncludedAssociation, 0, len(in)) + for _, ia := range in { + c := µflows.IncludedAssociation{ + Association: ia.Association, + IsParent: ia.IsParent, + IncludedAssociations: cloneIncludedAssociations(ia.IncludedAssociations, keepIDs), + } + if keepIDs { + c.ID = ia.ID + } + out = append(out, c) + } + return out +} diff --git a/mdl/executor/external_action_included_associations_test.go b/mdl/executor/external_action_included_associations_test.go new file mode 100644 index 0000000000..9674dac8f2 --- /dev/null +++ b/mdl/executor/external_action_included_associations_test.go @@ -0,0 +1,258 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "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/domainmodel" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// The reporter's shape (#1914, upstream mendixlabs/mxcli#1721): a command DTO +// with a sub-DTO, both entity types, so Studio Pro links them with an +// association and offers it under "Associations to include". +const includedAssocMetadata = ` + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +` + +// storedIncludedAssociations is the tree Studio Pro stored on ako/TestApp's +// Odata.IncludeAssociationsMF (11.16.0): Quantity ticked, Unit ticked under it. +func storedIncludedAssociations() []*microflows.IncludedAssociation { + unit := µflows.IncludedAssociation{Association: "Odata.Unit", IsParent: true} + unit.ID = "ia-unit" + qty := µflows.IncludedAssociation{ + Association: "Odata.Quantity", IsParent: true, + IncludedAssociations: []*microflows.IncludedAssociation{unit}, + } + qty.ID = "ia-qty" + return []*microflows.IncludedAssociation{qty} +} + +// storedCreateBufferCall is the call as the reader returns it from Studio Pro's +// document: the command mapping holds the tree, `additional` holds none. +func storedCreateBufferCall() *microflows.ActionActivity { + return µflows.ActionActivity{Action: µflows.CallExternalAction{ + ConsumedODataService: "Odata.Bug1914", + Name: "CreateBuffer", + ParameterMappings: []*microflows.ExternalActionParameterMapping{ + {ParameterName: "command", Argument: "$Cmd", ParameterEntity: "Odata.CreateBufferCommand", + IncludedAssociations: storedIncludedAssociations()}, + {ParameterName: "additional", Argument: "empty", CanBeEmpty: true}, + }, + }} +} + +// rewriteExternalActionFlow runs CREATE OR MODIFY of Odata.ACT_Buffer with body +// against a stored microflow holding stored (nil: there is no stored +// microflow), and returns the CallExternalActions the rewrite wrote. +func rewriteExternalActionFlow(t *testing.T, stored *microflows.ActionActivity, body string) []*microflows.CallExternalAction { + t.Helper() + mod := mkModule("Odata") + h := mkHierarchy(mod) + var existing []*microflows.Microflow + if stored != nil { + mf := mkMicroflow(mod.ID, "ACT_Buffer") + withContainer(h, mf.ContainerID, mod.ID) + mf.ObjectCollection = µflows.MicroflowObjectCollection{Objects: []microflows.MicroflowObject{stored}} + existing = append(existing, mf) + } + entity := func(name string) *domainmodel.Entity { + return &domainmodel.Entity{ + BaseElement: model.BaseElement{ID: nextID("ent")}, + Name: name, + RemoteServiceName: "Odata.Bug1914", + RemoteEntityName: name, + } + } + var written *microflows.Microflow + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { + return existing, nil + }, + ListDomainModelsFunc: func() ([]*domainmodel.DomainModel, error) { + return []*domainmodel.DomainModel{{ + ContainerID: mod.ID, + Entities: []*domainmodel.Entity{entity("CreateBufferCommand"), entity("QuantityParameterType"), entity("OtherCommand")}, + }}, nil + }, + ListConsumedODataServicesFunc: func() ([]*model.ConsumedODataService, error) { + return []*model.ConsumedODataService{{ + BaseElement: model.BaseElement{ID: "svc-1914"}, + ContainerID: mod.ID, + Name: "Bug1914", + Metadata: includedAssocMetadata, + }}, nil + }, + ListConsumedRestServicesFunc: func() ([]*model.ConsumedRestService, error) { return nil, nil }, + CreateMicroflowFunc: func(mf *microflows.Microflow) error { written = mf; return nil }, + UpdateMicroflowFunc: func(mf *microflows.Microflow) error { written = mf; return nil }, + } + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h)) + + prog, errs := visitor.Build("create or modify microflow Odata.ACT_Buffer ($Cmd: Odata.CreateBufferCommand, $Other: Odata.OtherCommand)\nbegin\n" + + body + "\nend;") + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + if err := execCreateMicroflow(ctx, prog.Statements[0].(*ast.CreateMicroflowStmt)); err != nil { + t.Fatalf("create or modify: %v", err) + } + if written == nil { + t.Fatal("nothing was written") + } + var calls []*microflows.CallExternalAction + for _, obj := range written.ObjectCollection.Objects { + if act, ok := obj.(*microflows.ActionActivity); ok { + if c, ok := act.Action.(*microflows.CallExternalAction); ok { + calls = append(calls, c) + } + } + } + return calls +} + +func mappingNamed(c *microflows.CallExternalAction, name string) *microflows.ExternalActionParameterMapping { + for _, pm := range c.ParameterMappings { + if pm.ParameterName == name { + return pm + } + } + return nil +} + +// #1914: MDL has no syntax for "Associations to include", so a CREATE OR MODIFY +// that restates the call wrote it with none, and the OData request silently +// lost the associated objects at runtime. The stored tree is carried onto the +// rewritten call, $IDs and nesting intact. +func TestCreateOrModify_CarriesIncludedAssociations(t *testing.T) { + calls := rewriteExternalActionFlow(t, storedCreateBufferCall(), + "call external action Odata.Bug1914.CreateBuffer(command = $Cmd, additional = empty);") + if len(calls) != 1 { + t.Fatalf("wrote %d calls, want 1", len(calls)) + } + cmd := mappingNamed(calls[0], "command") + if cmd == nil { + t.Fatal("no command mapping") + } + got := cmd.IncludedAssociations + if len(got) != 1 || got[0].Association != "Odata.Quantity" || !got[0].IsParent || got[0].ID != "ia-qty" || + len(got[0].IncludedAssociations) != 1 || got[0].IncludedAssociations[0].Association != "Odata.Unit" || + got[0].IncludedAssociations[0].ID != "ia-unit" { + t.Errorf("command mapping's associations to include were not carried: %+v", got) + } + if add := mappingNamed(calls[0], "additional"); add == nil || len(add.IncludedAssociations) != 0 { + t.Errorf("additional mapping gained associations: %+v", add) + } +} + +// Controls: the carry keeps what the statement still means, and nothing else. +func TestCreateOrModify_IncludedAssociationsCarryControls(t *testing.T) { + t.Run("parameter now binds another entity", func(t *testing.T) { + stored := storedCreateBufferCall() + stored.Action.(*microflows.CallExternalAction).Name = "CreateOther" + stored.Action.(*microflows.CallExternalAction).ParameterMappings[0].ParameterEntity = "Odata.CreateBufferCommand" + calls := rewriteExternalActionFlow(t, stored, + "call external action Odata.Bug1914.CreateOther(command = $Other);") + if cmd := mappingNamed(calls[0], "command"); cmd == nil || len(cmd.IncludedAssociations) != 0 { + t.Errorf("associations of CreateBufferCommand were carried onto an OtherCommand argument: %+v", cmd) + } + }) + t.Run("call to another action", func(t *testing.T) { + calls := rewriteExternalActionFlow(t, storedCreateBufferCall(), + "call external action Odata.Bug1914.CreateOther(command = $Other);") + if cmd := mappingNamed(calls[0], "command"); cmd == nil || len(cmd.IncludedAssociations) != 0 { + t.Errorf("a different action inherited CreateBuffer's associations: %+v", cmd) + } + }) + t.Run("new microflow", func(t *testing.T) { + calls := rewriteExternalActionFlow(t, nil, + "call external action Odata.Bug1914.CreateBuffer(command = $Cmd, additional = empty);") + if cmd := mappingNamed(calls[0], "command"); cmd == nil || len(cmd.IncludedAssociations) != 0 { + t.Errorf("a new call has associations nobody ticked: %+v", cmd) + } + }) +} + +// Two stored calls to one action: the carry cannot pair them by position, so it +// carries a tree they share and refuses when they differ. +func TestCreateOrModify_IncludedAssociationsTwoCalls(t *testing.T) { + twoCalls := func(second []*microflows.IncludedAssociation) *microflows.LoopedActivity { + b := storedCreateBufferCall() + b.Action.(*microflows.CallExternalAction).ParameterMappings[0].IncludedAssociations = second + return µflows.LoopedActivity{ObjectCollection: µflows.MicroflowObjectCollection{ + Objects: []microflows.MicroflowObject{storedCreateBufferCall(), b}, + }} + } + run := func(t *testing.T, stored *microflows.LoopedActivity) ([]*microflows.CallExternalAction, error) { + t.Helper() + var calls []*microflows.CallExternalAction + rebuilt := µflows.MicroflowObjectCollection{} + for range 2 { + a := storedCreateBufferCall() + a.Action.(*microflows.CallExternalAction).ParameterMappings[0].IncludedAssociations = nil + rebuilt.Objects = append(rebuilt.Objects, a) + calls = append(calls, a.Action.(*microflows.CallExternalAction)) + } + err := carryIncludedAssociations(rebuilt, + µflows.MicroflowObjectCollection{Objects: []microflows.MicroflowObject{stored}}, "Odata.ACT_Buffer") + return calls, err + } + + t.Run("same tree", func(t *testing.T) { + calls, err := run(t, twoCalls(storedIncludedAssociations())) + if err != nil { + t.Fatal(err) + } + a := calls[0].ParameterMappings[0].IncludedAssociations + b := calls[1].ParameterMappings[0].IncludedAssociations + if len(a) != 1 || len(b) != 1 || a[0].Association != "Odata.Quantity" || b[0].Association != "Odata.Quantity" { + t.Fatalf("both calls should keep the tree: %+v / %+v", a, b) + } + if a[0].ID != "ia-qty" || b[0].ID != "" || b[0].IncludedAssociations[0].ID != "" { + t.Errorf("the second copy must not reuse the stored $IDs: %q, %q", a[0].ID, b[0].ID) + } + }) + t.Run("different trees", func(t *testing.T) { + qtyOnly := storedIncludedAssociations() + qtyOnly[0].IncludedAssociations = nil + if _, err := run(t, twoCalls(qtyOnly)); err == nil || !strings.Contains(err.Error(), "Odata.Bug1914.CreateBuffer(command)") { + t.Errorf("want a refusal naming the call, got %v", err) + } + }) +} diff --git a/mdl/executor/flow_splice_negated_condition_pedapp_test.go b/mdl/executor/flow_splice_negated_condition_pedapp_test.go new file mode 100644 index 0000000000..ff4de1b66b --- /dev/null +++ b/mdl/executor/flow_splice_negated_condition_pedapp_test.go @@ -0,0 +1,168 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// ako/mxcli#1930 (mendixlabs/mxcli#1728): a decision whose true path goes +// straight on and whose false path holds the activities is printed by describe +// folded — `if not(X) then end if;` — with its paths swapped. Fed +// back through `create or modify` with only the condition changed, the in-place +// condition edit stored the script's `not(Y)` as the decision's expression +// while the true and false flows stayed where they were: the decision was +// inverted, and check, exec ("spliced: 1 edited in place") and diff all looked +// harmless. The stored expression must be the one that keeps the meaning the +// script states: the negation of the printed condition. + +const negatedSetup = `mdl 1; +create persistent entity MyFirstModule.Sale1930 (Code: String(200), Reference: String(200)); +create or modify microflow MyFirstModule.VAL_Sale1930 ($Sale: MyFirstModule.Sale1930) +returns Boolean +begin + declare $Valid Boolean = true; + @caption 'Code filled?' + if %s then + else + set $Valid = false; + end if; + return $Valid; +end;` + +// storedDecision returns the decision of VAL_Sale1930 and its expression. +func storedDecision(t *testing.T, exec *Executor) (*microflows.ExclusiveSplit, string) { + t.Helper() + mf := loopEditFlowOf(t, exec, "VAL_Sale1930") + for _, obj := range mf.ObjectCollection.Objects { + if s, ok := obj.(*microflows.ExclusiveSplit); ok { + cond, ok := s.SplitCondition.(*microflows.ExpressionSplitCondition) + if !ok { + t.Fatalf("the decision's condition is %T, want an expression", s.SplitCondition) + } + return s, cond.Expression + } + } + t.Fatal("VAL_Sale1930 has no decision") + return nil, "" +} + +func TestCreateOrModify_FoldedDecisionKeepsItsMeaning(t *testing.T) { + for _, tc := range []struct { + name, stored, printed, edited, want string + }{ + { + // The reporter's flow. + name: "printed as not(X)", + stored: "trim($Sale/Code) != ''", + printed: "if not(trim($Sale/Code) != '') then", + edited: "if not(trim($Sale/Reference) != '') then", + want: "trim($Sale/Reference) != ''", + }, + { + // describe takes a stored not(Z) out of the fold rather than + // printing not(not(Z)); the edit must put it back. + name: "stored not(Z), printed as Z", + stored: "not($Sale/Code = empty)", + printed: "if $Sale/Code = empty then", + edited: "if $Sale/Reference = empty then", + want: "not($Sale/Reference = empty)", + }, + } { + t.Run(tc.name, func(t *testing.T) { + exec, out, _ := openPedAppCopy(t) + if err := agreeExec(t, exec, strings.Replace(negatedSetup, "%s", tc.stored, 1)); err != nil { + t.Fatalf("setup: %v\n%s", err, out.String()) + } + before, _ := storedDecision(t, exec) + + described := describeFlow(t, exec, "VAL_Sale1930") + // Control: the flow is printed folded, which is the case this is about. + if !strings.Contains(described, tc.printed) { + t.Fatalf("control: describe does not print %q:\n%s", tc.printed, described) + } + src := strings.Replace(described, tc.printed, tc.edited, 1) + if !strings.HasPrefix(strings.TrimSpace(src), "mdl ") { + src = "mdl 1;\n" + src + } + out.Reset() + if err := agreeExec(t, exec, src); err != nil { + t.Fatalf("exec of the edited description: %v\n%s", err, out.String()) + } + + after, expr := storedDecision(t, exec) + if after.ID != before.ID { + t.Fatalf("the decision was replaced (%s → %s), not edited in place:\n%s", before.ID, after.ID, out.String()) + } + if expr != tc.want { + t.Fatalf("stored expression = %q, want %q: the decision's flows did not move, so storing the printed condition inverts it", expr, tc.want) + } + if again := describeFlow(t, exec, "VAL_Sale1930"); !strings.Contains(again, tc.edited) { + t.Errorf("describe no longer prints the edited condition %q:\n%s", tc.edited, again) + } + }) + } +} + +// The same fold inside a loop body, which describe prints from the loop's own +// collection. The splice does not change a decision in a loop body at all — it +// refuses, writing nothing — so the inversion cannot happen there; this pins +// that the folded condition is not stored by some other route. +func TestCreateOrModify_FoldedDecisionInALoopIsNotInverted(t *testing.T) { + const setup = `mdl 1; +create persistent entity MyFirstModule.Sale1930 (Code: String(200), Reference: String(200)); +create or modify microflow MyFirstModule.VAL_Loop1930 ($Sales: list of MyFirstModule.Sale1930) +returns Boolean +begin + declare $Valid Boolean = true; + loop $Sale in $Sales + begin + if trim($Sale/Code) != '' then + else + set $Valid = false; + end if; + end loop; + return $Valid; +end;` + const printed = "if not(trim($Sale/Code) != '') then" + const edited = "if not(trim($Sale/Reference) != '') then" + + exec, out, _ := openPedAppCopy(t) + if err := agreeExec(t, exec, setup); err != nil { + t.Fatalf("setup: %v\n%s", err, out.String()) + } + described := describeFlow(t, exec, "VAL_Loop1930") + if !strings.Contains(described, printed) { + t.Fatalf("control: describe does not print %q:\n%s", printed, described) + } + src := strings.Replace(described, printed, edited, 1) + if !strings.HasPrefix(strings.TrimSpace(src), "mdl ") { + src = "mdl 1;\n" + src + } + out.Reset() + if err := agreeExec(t, exec, src); err == nil || !strings.Contains(err.Error(), "cannot be spliced") { + t.Fatalf("exec of the edited description = %v, want the loop-body refusal\n%s", err, out.String()) + } + + var expr string + var walk func(oc *microflows.MicroflowObjectCollection) + walk = func(oc *microflows.MicroflowObjectCollection) { + for _, obj := range oc.Objects { + switch o := obj.(type) { + case *microflows.ExclusiveSplit: + if c, ok := o.SplitCondition.(*microflows.ExpressionSplitCondition); ok { + expr = c.Expression + } + case *microflows.LoopedActivity: + walk(o.ObjectCollection) + } + } + } + walk(loopEditFlowOf(t, exec, "VAL_Loop1930").ObjectCollection) + if want := "trim($Sale/Code) != ''"; expr != want { + t.Fatalf("stored expression in the loop = %q, want it unchanged, %q", expr, want) + } +} diff --git a/mdl/executor/register_stubs.go b/mdl/executor/register_stubs.go index 841fd0f313..a581b51921 100644 --- a/mdl/executor/register_stubs.go +++ b/mdl/executor/register_stubs.go @@ -28,6 +28,9 @@ func registerModuleHandlers(r *Registry) { r.Register(&ast.DropModuleStmt{}, func(ctx *ExecContext, stmt ast.Statement) error { return execDropModule(ctx, stmt.(*ast.DropModuleStmt)) }) + r.Register(&ast.AlterModuleSetStmt{}, func(ctx *ExecContext, stmt ast.Statement) error { + return execAlterModuleSet(ctx, stmt.(*ast.AlterModuleSetStmt)) + }) r.Register(&ast.AlterModuleJarDepStmt{}, func(ctx *ExecContext, stmt ast.Statement) error { return execAlterModuleJarDep(ctx, stmt.(*ast.AlterModuleJarDepStmt)) }) diff --git a/mdl/executor/registry_test.go b/mdl/executor/registry_test.go index 4ab7e4b321..09a3c7fbfa 100644 --- a/mdl/executor/registry_test.go +++ b/mdl/executor/registry_test.go @@ -168,6 +168,7 @@ func allKnownStatements() []ast.Statement { &ast.AlterEnumerationStmt{}, &ast.AlterKnowledgeBaseStmt{}, &ast.AlterModelStmt{}, + &ast.AlterModuleSetStmt{}, &ast.AlterModuleJarDepStmt{}, &ast.CreateNavigationStmt{}, &ast.AlterNavigationStmt{}, diff --git a/mdl/executor/stmt_summary.go b/mdl/executor/stmt_summary.go index e58247c47a..f08ad28a7c 100644 --- a/mdl/executor/stmt_summary.go +++ b/mdl/executor/stmt_summary.go @@ -35,6 +35,8 @@ func stmtSummary(stmt ast.Statement) string { return fmt.Sprintf("create module %s", s.Name) case *ast.DropModuleStmt: return fmt.Sprintf("drop module %s", s.Name) + case *ast.AlterModuleSetStmt: + return fmt.Sprintf("alter module %s set", s.ModuleName) case *ast.AlterModuleJarDepStmt: return fmt.Sprintf("alter module %s jar dep (%d actions)", s.ModuleName, len(s.Actions)) diff --git a/mdl/executor/validate_duplicates.go b/mdl/executor/validate_duplicates.go index bc144caa1a..bdf0d1fad8 100644 --- a/mdl/executor/validate_duplicates.go +++ b/mdl/executor/validate_duplicates.go @@ -107,7 +107,10 @@ func stmtCreateInfo(stmt ast.Statement) (docType, name string, idempotent bool) func stmtCreateKind(stmt ast.Statement) (docType, name string, idempotent bool) { switch s := stmt.(type) { case *ast.CreateModuleStmt: - return "module", s.Name, false + // `or modify` applies the module's documentation and settings to a + // module that exists (ako/mxcli#1921), so restating it is not a + // redefinition. + return "module", s.Name, s.CreateOrModify case *ast.CreateModuleRoleStmt: // Not a document, which is why it was missed when the document types // were swept (mendixlabs/mxcli#1067). exec refuses a plain CREATE of an diff --git a/mdl/executor/version_gates_coverage_1811_test.go b/mdl/executor/version_gates_coverage_1811_test.go index 21f91ac426..b93da9c617 100644 --- a/mdl/executor/version_gates_coverage_1811_test.go +++ b/mdl/executor/version_gates_coverage_1811_test.go @@ -142,6 +142,7 @@ var typeCoverageExceptions = map[string]string{ "Forms$SnippetParameterMapping": "unreachable: a snippet call maps only the parameters the called snippet declares, and a snippet holds parameters from 9.21 (Forms$Snippet.Parameters)", "CodeActions$CustomBlobDocumentParameterType": "unreachable: carried only from a stored Java action parameter of that type, which a project below 11.6 cannot hold", "Microflows$CustomBlobDocumentCodeActionParameterValue": "unreachable: written only for a Java action parameter of a custom document type, which a project below 11.6 cannot hold", + "Microflows$IncludedAssociation": "unreachable: carried only from a stored external action parameter's associations to include, which a project below 10.11 cannot hold (#1914)", // Residual: the statement does not say which shape the writer picks, so // no gate can; the write refuses it below its floor. "Forms$ToggleDesignPropertyValue": "lowered below 10.2 to a flat DesignPropertyValue (canon lowerDesignProperty)", diff --git a/mdl/executor/version_gates_documents.go b/mdl/executor/version_gates_documents.go index bda91c74c9..a6516dafa4 100644 --- a/mdl/executor/version_gates_documents.go +++ b/mdl/executor/version_gates_documents.go @@ -174,6 +174,27 @@ var documentTypeGates = []statementTypeGate{ return false }, }, + { + // An add-on module is ExportLevel Protected with ProtectedModuleType + // AddOn; below 9.12 there is no ProtectedModuleType to write + // (ako/mxcli#1921). + typeGate: typeGate{ + versionGate: versionGate{"modules", "module_type", "ModuleType: addon on a module", + "this version has no add-on modules (Projects$ModuleSettings.ProtectedModuleType): leave the module an app module, or upgrade the project"}, + types: []string{"Projects$ModuleSettings"}, + properties: []string{"Projects$ModuleSettings.ProtectedModuleType"}, + }, + applies: func(stmt ast.Statement) bool { + var c ast.ModuleSettingsClause + switch s := stmt.(type) { + case *ast.CreateModuleStmt: + c = s.Settings + case *ast.AlterModuleSetStmt: + c = s.Settings + } + return c.ModuleType != nil && *c.ModuleType == "addon" + }, + }, { typeGate: typeGate{ versionGate: versionGate{"modules", "jar_dependencies", "add jar dependency", diff --git a/mdl/executor/version_gates_documents_1811_test.go b/mdl/executor/version_gates_documents_1811_test.go index fc31a1bc76..2c33c24fa5 100644 --- a/mdl/executor/version_gates_documents_1811_test.go +++ b/mdl/executor/version_gates_documents_1811_test.go @@ -24,6 +24,7 @@ func TestCheckVersionGates_DocumentShapes1811(t *testing.T) { } iconItem := ast.NavMenuItemDef{Caption: "Home", Icon: "Atlas_Core.Atlas_Filled.home", IconKind: types.MenuIconCollection} getOp := &ast.RestOperationDef{Name: "Get", Method: "GET", Path: "/x"} + addon := "addon" queryOp := &ast.RestOperationDef{Name: "Search", Method: "GET", Path: "/s", QueryParameters: []ast.RestParamDef{{Name: "q", DataType: "String"}}} checkStatementGateCases(t, []statementGateCase{ {"create snippet with a parameter", &ast.CreateSnippetStmtV3{Name: qn("M", "S"), Parameters: param}, @@ -66,6 +67,10 @@ func TestCheckVersionGates_DocumentShapes1811(t *testing.T) { {"add jar dependency", &ast.AlterModuleJarDepStmt{ModuleName: "M", Actions: []ast.JarDepAction{ &ast.AddJarDepAction{Group: "g", Artifact: "a", Version: "1"}}}, [3]int{9, 24, 30}, [3]int{10, 0, 0}, "add jar dependency"}, + {"create an add-on module", &ast.CreateModuleStmt{Name: "M", Settings: ast.ModuleSettingsClause{ModuleType: &addon}}, + [3]int{9, 11, 0}, [3]int{9, 12, 0}, "ModuleType: addon on a module"}, + {"make a module an add-on", &ast.AlterModuleSetStmt{ModuleName: "M", Settings: ast.ModuleSettingsClause{ModuleType: &addon}}, + [3]int{9, 11, 0}, [3]int{9, 12, 0}, "ModuleType: addon on a module"}, {"add a jar dependency exclusion", &ast.AlterModuleJarDepStmt{ModuleName: "M", Actions: []ast.JarDepAction{ &ast.AddJarDepExclusionAction{}}}, [3]int{10, 11, 0}, [3]int{10, 12, 0}, "add exclusion on a jar dependency"}, @@ -100,6 +105,7 @@ func TestCheckVersionGates_DocumentShapes1811(t *testing.T) { // variables, a menu item without an icon, a REST client with no operation. func TestCheckVersionGates_PlainShapesAreNotGated(t *testing.T) { qn := func(module, name string) ast.QualifiedName { return ast.QualifiedName{Module: module, Name: name} } + app, version := "app", "2.0.0" for _, stmt := range []ast.Statement{ &ast.CreateWorkflowStmt{Name: qn("M", "WF"), Activities: []ast.WorkflowActivityNode{&ast.WorkflowCallMicroflowNode{Microflow: qn("M", "Act")}}}, &ast.CreateSnippetStmtV3{Name: qn("M", "S")}, @@ -108,6 +114,8 @@ func TestCheckVersionGates_PlainShapesAreNotGated(t *testing.T) { &ast.CreateNavigationStmt{ProfileName: "Responsive", ProgressiveWebApp: &types.NavPWASpec{Off: true}}, &ast.CreateNavigationStmt{ProfileName: "Responsive", HomePages: []ast.NavHomePageDef{{IsPage: true, Target: qn("M", "Home")}}}, &ast.AlterModuleJarDepStmt{ModuleName: "M", Actions: []ast.JarDepAction{&ast.DropJarDepAction{}}}, + // An app module and a version are in every version's metamodel. + &ast.AlterModuleSetStmt{ModuleName: "M", Settings: ast.ModuleSettingsClause{ModuleType: &app, Version: &version}}, } { prog := &ast.Program{Statements: []ast.Statement{&ast.CreateModuleStmt{Name: "M"}, stmt}} if vs := versionGateViolations(versionCtx(t, 9, 0, 2), prog); len(vs) != 0 { diff --git a/mdl/grammar/MDLLexer.g4 b/mdl/grammar/MDLLexer.g4 index 50deaf6dbe..c4e15a7d7a 100644 --- a/mdl/grammar/MDLLexer.g4 +++ b/mdl/grammar/MDLLexer.g4 @@ -27,6 +27,35 @@ func HasStrictEscapes(in antlr.CharStream) bool { return ok } +// followsMemberSeparator reports whether the token just matched comes straight +// after a `/` or `.` member separator on the same line (spaces and tabs between +// them skipped), as `Error` does in `Obj/Error`. A block comment's closing `*/` +// is not a separator, and neither is a line break: a line comment ending in a +// full stop (`-- see the docs.`) must not swallow the `error message` clause on +// the next line. +// +// It is a right-edge predicate, so it runs only once the input has matched the +// rule; at the left edge ANTLR evaluates it at the start of every token, which +// disables the lexer's DFA cache and doubled `check` time on the examples. The +// token's start is therefore read from TokenStartCharIndex: LA(j) for a +// negative j is the character at Index()+j. +func (l *MDLLexer) followsMemberSeparator() bool { + in := l.GetInputStream() + at := func(k int) int { return in.LA(k - in.Index()) } + for k := l.TokenStartCharIndex - 1; k >= 0; k-- { + switch at(k) { + case ' ', '\t': + continue + case '.': + return true + case '/': + return k == 0 || at(k-1) != '*' + } + return false + } + return false +} + // isTrailingComma reports whether the ',' just matched ends a bracketed list: // the next significant character closes a (), {} or [] (whitespace and // comments skipped), and the one before it neither opens a list nor is another @@ -194,9 +223,15 @@ RESTRICT: R E S T R I C T; // // `error message` is the one spelling (R8, ako/mxcli#752); the other two are // aliases, and so are `error`, `feedback` where a rule takes a message. -ERROR_MESSAGE: E R R O R WS+ M E S S A G E +// +// Never right after a `/` or `.` member separator: there `Error` is a member's +// name, so `validation feedback $E/Error message 'x'` — what describe writes +// for an attribute called Error — is the attribute and then the MESSAGE +// keyword, not this clause (mendixlabs/mxcli#1720). +ERROR_MESSAGE: ( E R R O R WS+ M E S S A G E | E R R O R '_' M E S S A G E /* @alias MDL-DEPR021 */ - | E R R O R M E S S A G E /* @alias MDL-DEPR021 */; + | E R R O R M E S S A G E /* @alias MDL-DEPR021 */ + ) {!p.followsMemberSeparator()}?; CASCADE: C A S C A D E; PREVENT: P R E V E N T; diff --git a/mdl/grammar/MDLParser.g4 b/mdl/grammar/MDLParser.g4 index 0c23ffa521..8b4576dfb5 100644 --- a/mdl/grammar/MDLParser.g4 +++ b/mdl/grammar/MDLParser.g4 @@ -291,6 +291,7 @@ alterStatement | ALTER KNOWLEDGE BASE qualifiedName SET agentEditorAlterAssignment (COMMA agentEditorAlterAssignment)* | ALTER CONSUMED MCP SERVICE qualifiedName SET agentEditorAlterAssignment (COMMA agentEditorAlterAssignment)* | ALTER AGENT qualifiedName alterAgentAction+ + | alterModuleSetStatement | alterModuleJarDepStatement ; diff --git a/mdl/grammar/domains/MDLDomainModel.g4 b/mdl/grammar/domains/MDLDomainModel.g4 index 6acff8291c..8d0ae1e24b 100644 --- a/mdl/grammar/domains/MDLDomainModel.g4 +++ b/mdl/grammar/domains/MDLDomainModel.g4 @@ -369,7 +369,25 @@ alterEnumerationAction // ============================================================================= createModuleStatement - : MODULE ifNotExists? identifierOrKeyword moduleOptions? + : MODULE ifNotExists? identifierOrKeyword moduleOptions? moduleSettingsBody? + ; + +// The General tab of Studio Pro's Module settings dialog, as a property list: +// `( Version: '2.3.1', ModuleType: addon )` (ako/mxcli#1921). The visitor +// holds the key table; `alter module … set ( … )` takes the same keys. +moduleSettingsBody + : LPAREN (moduleSettingProperty (COMMA moduleSettingProperty)* COMMA?)? RPAREN + ; + +// NUMBER_LITERAL only so `Version: 2.0` gets the visitor's message, not a +// parse error. +moduleSettingProperty + : identifierOrKeyword COLON (STRING_LITERAL | NUMBER_LITERAL | identifierOrKeyword) + ; + +// ALTER MODULE name SET ( Version: '…', ModuleType: app | addon ) +alterModuleSetStatement + : ALTER MODULE (qualifiedName | IDENTIFIER) SET moduleSettingsBody ; // ============================================================================= diff --git a/mdl/visitor/error_member_name_test.go b/mdl/visitor/error_member_name_test.go new file mode 100644 index 0000000000..f4440989de --- /dev/null +++ b/mdl/visitor/error_member_name_test.go @@ -0,0 +1,80 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// A member named `Error` followed by the `message` keyword lexed as the one +// compound ERROR_MESSAGE token, so `validation feedback $E/Error message 'x'` +// — exactly what describe writes for such an attribute — failed with "missing +// MESSAGE" (mendixlabs/mxcli#1720). After a `/` or `.` member separator the +// word is a name, never the start of the `error message` clause. +func TestValidationFeedbackOnMemberNamedError(t *testing.T) { + cases := map[string]struct { + target string + wantLast string + }{ + "attribute Error": {"$E/Error", "Error"}, + "attribute error": {"$E/error", "error"}, + "attribute ERROR": {"$E/ERROR", "ERROR"}, + "after an association": {"$E/M.E_Other/Error", "Error"}, + "spaced separator": {"$E / Error", "Error"}, + "qualified with dot": {"$E/M.Other.Error", "M.Other.Error"}, + "attribute ErrorMessage": {"$E/ErrorMessage", "ErrorMessage"}, + "attribute Error_Message": {"$E/Error_Message", "Error_Message"}, + "control: unrelated member": {"$E/Code", "Code"}, + } + for name, tc := range cases { + for _, header := range []string{"", "mdl 1;\n"} { + t.Run(name+"/"+strings.TrimSpace(header), func(t *testing.T) { + src := header + "create or modify microflow M.ACT ($E: M.E)\nbegin\n" + + " validation feedback " + tc.target + " message 'not good';\n" + + " return;\nend;" + prog, errs := Build(src) + if len(errs) > 0 { + t.Fatalf("must parse: %v\n%s", errs, src) + } + mf, ok := prog.Statements[len(prog.Statements)-1].(*ast.CreateMicroflowStmt) + if !ok { + t.Fatalf("want a microflow, got %T", prog.Statements[len(prog.Statements)-1]) + } + vf, ok := mf.Body[0].(*ast.ValidationFeedbackStmt) + if !ok { + t.Fatalf("want validation feedback, got %T", mf.Body[0]) + } + segs := vf.AttributePath.Segments + if len(segs) == 0 || segs[len(segs)-1].Name != tc.wantLast { + t.Fatalf("last segment = %v, want %q", segs, tc.wantLast) + } + if vf.Message == nil { + t.Fatal("the message was lost") + } + }) + } + } +} + +// The `error message` clause itself must keep lexing as ERROR_MESSAGE wherever +// it follows something other than a member separator — including a block +// comment, whose closing `*/` ends in a slash. +func TestErrorMessageClauseStillLexes(t *testing.T) { + for name, src := range map[string]string{ + "attribute validation": "create entity M.E (Name: String(100) not null error message 'Required');", + "on delete restrict": "create association M.A_B from M.A to M.B on delete restrict error message 'In use';", + "after a block comment": "create association M.A_B from M.A to M.B on delete restrict /* why */ error message 'In use';", + "after a spaced comment": "create association M.A_B from M.A to M.B on delete restrict /* why */\n error message 'In use';", + "after a line comment": "create association M.A_B from M.A to M.B on delete restrict -- see the docs.\n error message 'In use';", + "after a path comment": "create association M.A_B from M.A to M.B on delete restrict -- see a/b/\n error message 'In use';", + } { + t.Run(name, func(t *testing.T) { + if _, errs := Build(src); len(errs) > 0 { + t.Fatalf("must parse: %v\n%s", errs, src) + } + }) + } +} diff --git a/mdl/visitor/visitor_module.go b/mdl/visitor/visitor_module.go index 43ad7a99e5..73aa74ff16 100644 --- a/mdl/visitor/visitor_module.go +++ b/mdl/visitor/visitor_module.go @@ -19,6 +19,9 @@ func (b *Builder) ExitCreateModuleStatement(ctx *parser.CreateModuleStatementCon // The doc comment is the domain model's documentation, the only // documentation a module has (mendixlabs/mxcli#1314). stmt.Documentation, stmt.DocumentationSet = findDocComment(ctx) + if body := ctx.ModuleSettingsBody(); body != nil { + stmt.Settings = b.moduleSettings("module "+name, body) + } b.statements = append(b.statements, stmt) } diff --git a/mdl/visitor/visitor_module_settings.go b/mdl/visitor/visitor_module_settings.go index 6ebddac9ff..30ef3652b8 100644 --- a/mdl/visitor/visitor_module_settings.go +++ b/mdl/visitor/visitor_module_settings.go @@ -3,6 +3,8 @@ package visitor import ( + "fmt" + "regexp" "strings" "github.com/mendixlabs/mxcli/mdl/ast" @@ -130,3 +132,91 @@ func parseJarDepAction(ctx *parser.AlterModuleJarDepActionContext) ast.JarDepAct return nil } + +// moduleSettingKeys are the keys of a module's settings list, named after the +// fields of Studio Pro's Module settings dialog (ako/mxcli#1921). +var moduleSettingKeys = []string{"Version", "ModuleType"} + +// moduleSettingsVersion is the dialog's rule for a module version: SemVer +// major.minor.patch, with an optional pre-release and build suffix. +var moduleSettingsVersion = regexp.MustCompile(`^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(-[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?(\+[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?$`) + +// moduleSettings reads a `( Version: '…', ModuleType: … )` list. An unknown +// key, a repeated key or a value of the wrong shape is an error here, so check +// and exec refuse the same statements. +func (b *Builder) moduleSettings(what string, body parser.IModuleSettingsBodyContext) ast.ModuleSettingsClause { + var out ast.ModuleSettingsClause + bc, ok := body.(*parser.ModuleSettingsBodyContext) + if !ok || bc == nil { + return out + } + seen := map[string]bool{} + for _, p := range bc.AllModuleSettingProperty() { + pc, ok := p.(*parser.ModuleSettingPropertyContext) + if !ok || pc == nil || pc.IdentifierOrKeyword(0) == nil { + continue + } + line := pc.GetStart().GetLine() + key := identifierOrKeywordText(pc.IdentifierOrKeyword(0)) + canonical := canonicalKey(key, moduleSettingKeys) + if canonical == "" { + b.addError(unknownKeyError(line, what, key, moduleSettingKeys)) + continue + } + if seen[canonical] { + b.addError(fmt.Errorf("line %d: %s sets %s twice", line, what, canonical)) + continue + } + seen[canonical] = true + switch canonical { + case "Version": + lit := pc.STRING_LITERAL() + if lit == nil { + b.addError(fmt.Errorf("line %d: %s: Version takes a string, as in Version: '1.2.0'", line, what)) + continue + } + v := unquoteStringLit(lit) + if !moduleSettingsVersion.MatchString(v) { + b.addError(fmt.Errorf("line %d: %s: Version '%s' is not a SemVer version: Studio Pro takes major.minor.patch, as in '1.2.0'", line, what, v)) + continue + } + out.Version = &v + case "ModuleType": + var t string + if pc.STRING_LITERAL() == nil && pc.IdentifierOrKeyword(1) != nil { + t = strings.ToLower(identifierOrKeywordText(pc.IdentifierOrKeyword(1))) + } + if t != "app" && t != "addon" { + b.addError(fmt.Errorf("line %d: %s: ModuleType takes app or addon, not %s", line, what, nodeText(pc.GetChild(2)))) + continue + } + out.ModuleType = &t + } + } + return out +} + +// ExitAlterModuleSetStatement handles ALTER MODULE name SET ( Key: value, … ). +func (b *Builder) ExitAlterModuleSetStatement(ctx *parser.AlterModuleSetStatementContext) { + var moduleName string + if qn := ctx.QualifiedName(); qn != nil { + moduleName = getQualifiedNameText(qn) + } else if id := ctx.IDENTIFIER(); id != nil { + moduleName = id.GetText() + } + if moduleName == "" { + return + } + stmt := &ast.AlterModuleSetStmt{ModuleName: moduleName} + body := ctx.ModuleSettingsBody() + if bc, ok := body.(*parser.ModuleSettingsBodyContext); ok && bc != nil && len(bc.AllModuleSettingProperty()) == 0 { + b.addError(fmt.Errorf("line %d: alter module %s set () names no setting: it takes %s", + ctx.GetStart().GetLine(), moduleName, strings.Join(moduleSettingKeys, ", "))) + return + } + stmt.Settings = b.moduleSettings("module "+moduleName, body) + if stmt.Settings.IsEmpty() { + return // every key was refused above + } + b.statements = append(b.statements, stmt) +} diff --git a/mdl/visitor/visitor_module_settings_test.go b/mdl/visitor/visitor_module_settings_test.go new file mode 100644 index 0000000000..51948c3228 --- /dev/null +++ b/mdl/visitor/visitor_module_settings_test.go @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "reflect" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// ako/mxcli#1921: the General tab of Studio Pro's Module settings dialog as a +// property list on `create module` and `alter module … set`. +func TestModuleSettings_Parse(t *testing.T) { + str := func(s string) *string { return &s } + for _, tc := range []struct { + src string + want ast.Statement + }{{ + src: "create or modify module Inventory (Version: '2.3.1', ModuleType: addon,);", + want: &ast.CreateModuleStmt{Name: "Inventory", CreateOrModify: true, + Settings: ast.ModuleSettingsClause{Version: str("2.3.1"), ModuleType: str("addon")}}, + }, { + // Keys and the type are case-insensitive; the type is stored lower-case. + src: "alter module Inventory set (moduletype: App);", + want: &ast.AlterModuleSetStmt{ModuleName: "Inventory", Settings: ast.ModuleSettingsClause{ModuleType: str("app")}}, + }, { + src: "alter module Inventory set (Version: '1.0.0-rc.1+build.5');", + want: &ast.AlterModuleSetStmt{ModuleName: "Inventory", Settings: ast.ModuleSettingsClause{Version: str("1.0.0-rc.1+build.5")}}, + }, { + // Control: no list states no setting, so a rerun keeps what is stored. + src: "create or modify module Inventory;", + want: &ast.CreateModuleStmt{Name: "Inventory", CreateOrModify: true}, + }} { + t.Run(tc.src, func(t *testing.T) { + prog := mustBuild(t, tc.src) + if len(prog.Statements) != 1 || !reflect.DeepEqual(prog.Statements[0], tc.want) { + t.Fatalf("got %#v, want %#v", prog.Statements, tc.want) + } + }) + } +} + +// What the dialog would refuse is refused here, so check and exec agree. +func TestModuleSettings_Refused(t *testing.T) { + for src, want := range map[string]string{ + "alter module M set (Versoin: '1.0.0');": "did you mean 'Version'", + "alter module M set (Version: '1.0.0', Version: '1.0.1');": "sets Version twice", + "alter module M set (Version: '2.3');": "not a SemVer version", + "alter module M set (Version: 2);": "Version takes a string", + "alter module M set (ModuleType: solution);": "ModuleType takes app or addon", + "alter module M set (ModuleType: 'addon');": "ModuleType takes app or addon", + "alter module M set ();": "names no setting", + "create module M (Location: marketplace);": "has no property 'Location'", + } { + t.Run(src, func(t *testing.T) { + _, errs := Build(src) + if len(errs) == 0 || !strings.Contains(errs[0].Error(), want) { + t.Errorf("errors = %v, want one containing %q", errs, want) + } + }) + } +} diff --git a/scripts/test-tunnel-seam.sh b/scripts/test-tunnel-seam.sh new file mode 100755 index 0000000000..9842f015ed --- /dev/null +++ b/scripts/test-tunnel-seam.sh @@ -0,0 +1,42 @@ +#!/usr/bin/env bash +# Run the tunnel seam's !linux stub tests on a non-Linux runner (ADR-0009). +# Shared by push-test.yml (Windows on PRs, Windows + macOS on main) and +# nightly.yml (macOS), so the two cannot drift. +# +# Scoped with -run to the seam's own tests. The full test binaries are still +# COMPILED for this platform, so a Windows/macOS build break is still caught; +# only the !linux stub behaviour is executed. +# +# Running the whole packages here fails on Windows for reasons that predate +# this check and are unrelated to the tunnel: several tests assert POSIX file +# modes (0600) that Windows does not implement — os.Chmod only toggles the +# read-only bit, so Stat reports 666 — plus one path-separator assumption. +# Tracked separately in #897. +# +# -run can pass vacuously if the tests are renamed or deleted, so assert that +# the expected number actually ran. +# +# `tee` rather than a command substitution: under `bash -e -o pipefail`, +# capturing into `out=$(go test ...)` aborts AT THE ASSIGNMENT when go test +# fails, and the output is never printed. The log then holds nothing but +# "Process completed with exit code 1" — no test name, no failure message, no +# way to tell a real break from a flake. Write the output as it is produced, +# then judge it. +set -uo pipefail + +out=go-test-output.txt +go test -v -count=1 -run 'Unsupported' ./cmd/mxcli/docker/... ./cmd/mxcli/tunnelhub/... 2>&1 | tee "$out" +status=${PIPESTATUS[0]} + +n=$(grep -c '^--- PASS: Test.*Unsupported' "$out" || true) +echo "seam tests executed: $n" +if [ "$status" -ne 0 ]; then + echo "FAIL: go test exited $status. Its output is above." + exit "$status" +fi +if [ "$n" -lt 4 ]; then + echo "FAIL: expected at least 4 tunnel-seam tests to run, -run matched $n." + echo " The !linux stubs in cmd/mxcli/docker and cmd/mxcli/tunnelhub" + echo " must each keep a test whose name contains 'Unsupported'." + exit 1 +fi diff --git a/sdk/microflows/microflows_actions.go b/sdk/microflows/microflows_actions.go index f752c725f4..a071a98adc 100644 --- a/sdk/microflows/microflows_actions.go +++ b/sdk/microflows/microflows_actions.go @@ -918,6 +918,23 @@ type ExternalActionParameterMapping struct { // type-checked against a parameter that has no type (mendixlabs/mxcli#1020). ParameterDataType string `json:"parameterDataType,omitempty"` ParameterEntity string `json:"parameterEntity,omitempty"` + // IncludedAssociations is Studio Pro's "Associations to include" for an + // object parameter: the associations whose objects are sent with it in the + // OData request. It is a tree, not a flag — an association ticked under + // another is stored nested in it. Without it the request carries only the + // object's own attributes (#1914). + IncludedAssociations []*IncludedAssociation `json:"includedAssociations,omitempty"` +} + +// IncludedAssociation is one ticked association under an external action +// parameter (Microflows$IncludedAssociation). +type IncludedAssociation struct { + model.BaseElement + Association string `json:"association,omitempty"` // BY_NAME reference + // IsParent is true when the object on this side of the tree is the + // association's owner (its ParentPointer, the FROM entity). + IsParent bool `json:"isParent,omitempty"` + IncludedAssociations []*IncludedAssociation `json:"includedAssociations,omitempty"` } // WebServiceCallAction calls a web service. diff --git a/sdk/versions/mendix-10.yaml b/sdk/versions/mendix-10.yaml index 81e4af2b87..b4cdf3ed24 100644 --- a/sdk/versions/mendix-10.yaml +++ b/sdk/versions/mendix-10.yaml @@ -423,6 +423,10 @@ features: min_version: "10.12.0" mdl: "ALTER MODULE Module SET JAR DEPENDENCY 'group:artifact' ADD EXCLUSION '…'" notes: "Projects$JarDependencyExclusion introduced in 10.12.0 (mendixmodelsdk 4.117.0; ako/mxcli#1811)." + module_type: + min_version: "9.12.0" + mdl: "CREATE MODULE M ( ModuleType: addon ) / ALTER MODULE M SET ( ModuleType: addon )" + notes: "An add-on module is ModuleSettings.ExportLevel = Protected with ProtectedModuleType = AddOn; ProtectedModuleType introduced in 9.12.0 (mendixmodelsdk 4.117.0; ako/mxcli#1921)." scheduled_events: schedules: diff --git a/sdk/versions/mendix-11.yaml b/sdk/versions/mendix-11.yaml index 2d7727fb88..5c64a4c400 100644 --- a/sdk/versions/mendix-11.yaml +++ b/sdk/versions/mendix-11.yaml @@ -505,6 +505,10 @@ features: min_version: "10.12.0" mdl: "ALTER MODULE Module SET JAR DEPENDENCY 'group:artifact' ADD EXCLUSION '…'" notes: "Projects$JarDependencyExclusion introduced in 10.12.0 (mendixmodelsdk 4.117.0; ako/mxcli#1811)." + module_type: + min_version: "9.12.0" + mdl: "CREATE MODULE M ( ModuleType: addon ) / ALTER MODULE M SET ( ModuleType: addon )" + notes: "An add-on module is ModuleSettings.ExportLevel = Protected with ProtectedModuleType = AddOn; ProtectedModuleType introduced in 9.12.0 (mendixmodelsdk 4.117.0; ako/mxcli#1921)." scheduled_events: schedules: diff --git a/sdk/versions/mendix-9.yaml b/sdk/versions/mendix-9.yaml index 19a8dd4565..d7be45525c 100644 --- a/sdk/versions/mendix-9.yaml +++ b/sdk/versions/mendix-9.yaml @@ -172,6 +172,12 @@ features: mdl: "MENU ITEM 'Home' ( OnClick: …, Icon: Module.Collection.icon )" notes: "Forms$IconCollectionIcon introduced in 9.20.0 with icon collections (mendixmodelsdk 4.117.0); `Icon:` names an icon collection entry and nothing else (ako/mxcli#1811)." + modules: + module_type: + min_version: "9.12.0" + mdl: "CREATE MODULE M ( ModuleType: addon ) / ALTER MODULE M SET ( ModuleType: addon )" + notes: "An add-on module is ModuleSettings.ExportLevel = Protected with ProtectedModuleType = AddOn; ProtectedModuleType introduced in 9.12.0 (mendixmodelsdk 4.117.0; ako/mxcli#1921)." + scheduled_events: schedules: min_version: "9.12.0"