diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-check-several-files-ends-on-check-passed-after-failure.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-check-several-files-ends-on-check-passed-after-failure.json new file mode 100644 index 0000000000..dc40fb8935 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-check-several-files-ends-on-check-passed-after-failure.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-12", "symptom": "`mxcli check -p app.mpr bad.mdl good.mdl` exits 1 but its last line is \"Check passed!\"; reversing the file order flips the last line, so anything reading the log tail sees success", "cause": "`runCheckFiles` ran `runCheckFileAgainst` per file and kept only the worst exit code; each file prints its own verdict and nothing printed one for the set, so the last file's \"Check passed!\" (plus its no-project note) closed the run", "file": "`cmd/mxcli/cmd_check.go` (`runCheckFiles`)", "insight": "A per-unit verdict line is read as the run's verdict by whoever tails the log (agents especially); when a command loops over inputs, the loop owner must print the aggregate last. The per-file \"Check passed!\" is followed by the two-line no-project note, so assert on the true last line rather than `Contains`, or the test passes against the bug. No project needed for the test: a syntax error makes the failing file", "refs": ["mendixlabs/mxcli#1431"]} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-describe-module-help-promises-all-contents.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-describe-module-help-promises-all-contents.json new file mode 100644 index 0000000000..5ccc584d8c --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-describe-module-help-promises-all-contents.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-12", "symptom": "`mxcli describe module Banking -p app.mpr` prints only `create or modify module Banking;` and its module roles, although `describe --help` says \"module Describe a module (all contents)\"; the full output needs `-c \"describe module Banking with all\"`, documented only in the changelog", "cause": "describeMDLCommand maps MODULE to a plain DESCRIBE MODULE (module + roles, by design of the MDL statement); the help line described WITH ALL, which the subcommand had no way to request", "file": "cmd/mxcli/cmd_describe.go, cmd/mxcli/main.go", "fix": "Added `--all` (applyDescribeAll appends WITH ALL; refused on any other type) and made the help say what plain describe module prints", "insight": "Fixed the help, not the default: a plain describe module matches `-c \"describe module X\"` and existing scripts that describe a module to get its header, and WITH ALL has known open gaps (#1421 omits doc types, #1425 truncates at 10k lines silently), so silently switching the default would trade a wrong help line for silently incomplete output. When a subcommand's help and the MDL statement disagree, check which one the -c form agrees with before choosing which to change.", "refs": ["mendixlabs/mxcli#1435"]} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-describe-subcommand-rejects-rule.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-describe-subcommand-rejects-rule.json new file mode 100644 index 0000000000..a2cefa9469 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-describe-subcommand-rejects-rule.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-12", "symptom": "`mxcli describe rule M.Rule_X` prints \"Unknown type: rule\" and bare `mxcli describe M.Rule_X` says no describable document is found, while `mxcli -c \"describe rule M.Rule_X\"` works", "cause": "Same drift as #1347: describeMDLCommand had no RULE case, and the auto-detect maps (catalog ObjectType RULE, unit $Type Microflows$Rule) omitted rules — the unit map even said so on purpose, a leftover from before the executor could describe rules", "file": "cmd/mxcli/cmd_describe.go", "fix": "Added RULE -> DESCRIBE RULE, objectTypeToDescribe[RULE] and unitTypeToDescribe[Microflows$Rule] = rule, plus the help and Valid-types lines", "insight": "Second instance of the subcommand's hand-kept type list lagging the DESCRIBE grammar. The executor's own bare-DESCRIBE map (mdl/executor/describe_auto.go objectTypeToDescribeKind) already had RULE — diffing that map's keys against cmd/mxcli's objectTypeToDescribe is the quickest way to find the next missing doctype. The 'not a rule in this project' refusal in the same report is a downstream effect, not a describe bug.", "refs": ["mendixlabs/mxcli#1402"]} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-marketplace-download-output-directory-rename-file-exists.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-marketplace-download-output-directory-rename-file-exists.json new file mode 100644 index 0000000000..a1b13ba58d --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-marketplace-download-output-directory-rename-file-exists.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-12", "symptom": "`mxcli marketplace download --output ` fails with `write : rename /.mxcli-download-*.part : file exists`; only a file path works", "cause": "The command streams to a temp file in `filepath.Dir(output)` and renames it onto `output`, treating `--output` as a file path unconditionally; rename(2) onto a non-empty directory is EEXIST", "file": "`cmd/mxcli/cmd_marketplace.go` (`runMarketplaceDownload`)", "insight": "The temp-then-rename pattern decides the destination before it knows what `--output` names. Resolve directory-ness first (existing dir, or trailing separator = dir to create), put the temp file inside it so the rename stays on one filesystem, and join the CDN filename (or the content--.mpk fallback). Testable offline: `runMarketplace` + `marketplaceClientFactory` with one httptest server serving versions, the 303 hop and the CDN file — the CDN hop uses http.DefaultClient, which reaches a plain-http httptest server fine", "refs": ["mendixlabs/mxcli#1443"]} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-run-local-mendix9-defaults-to-jdk21.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-run-local-mendix9-defaults-to-jdk21.json new file mode 100644 index 0000000000..00a321c883 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-run-local-mendix9-defaults-to-jdk21.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-12", "symptom": "`mxcli run --local --db-type hsqldb` on a Mendix 9.24 app failed in mxbuild's Gradle 7.6 with \"Unsupported class file major version 65\": mxcli resolved JDK 21 and skipped a JAVA_HOME pointing at JDK 17, and the JDK-not-found hint suggested `alter settings runtime ( JavaVersion: '21' )`, which no-ops on 9.x", "cause": "`ProjectJavaMajor` fell back to a flat `DefaultJavaMajor = 21` whenever the model stores no JavaVersion — but a Mendix 9 model never stores one (the setting does not exist before 10.x), and 9.x supports Java 11/17 only. The detect.go hint fired for any major != 21, including 9.x's 17", "file": "`cmd/mxcli/docker/javaversion.go` (`ProjectJavaMajor`, `defaultJavaMajorForMendix`), `cmd/mxcli/docker/detect.go` (`resolveJDKWith` hint gate)", "insight": "\"Setting absent\" meant \"use the platform default\" only while every supported version shared one default; once a version range without the setting has a different JDK, the fallback has to be keyed on the Mendix version, which the reader already has (`ProjectVersion().MajorVersion`). The committed 9.24.30 fixture (modelsdk/mpr/testdata/v1-project) made this testable without mxbuild. Revert proof: ProjectJavaMajor(9.24 fixture) = (21, false), want (17, false)", "refs": ["mendixlabs/mxcli#1437"]} diff --git a/.claude/skills/fix-issue/findings/mdl-backend/2026-10-12-flow-writes-refused-on-mendix-9.json b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-12-flow-writes-refused-on-mendix-9.json new file mode 100644 index 0000000000..bf96fb6dcc --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-12-flow-writes-refused-on-mendix-9.json @@ -0,0 +1 @@ +{"area": "mdl/backend", "date": "2026-10-12", "refs": ["mendixlabs/mxcli#1410", "mendixlabs/mxcli#1434"], "symptom": "Every create microflow / create nanoflow / alter microflow on a Mendix 9.x project failed after earlier statements had already been written: \"this Mendix 9.24.2 project cannot store Microflows$Microflow.StableId (Mendix 10.2.0+); Microflows$SequenceFlow.CaseValues (Mendix 10.15.0+)\" (10.0-10.14: CaseValues alone), while `mxcli check` passed the script", "cause": "The flow writers emitted the newest shape whatever the version, gated by major at most: StableId as an unconditional FreshGUIDFields codec default (10.2+), the case always as a CaseValues list (10.15+), always a BezierCurve Line (10.16+), ReturnVariableName and parameter IsRequired/DefaultValue on any 10.x (10.12+ / 10.17+). The property floor added on 2026-10-10 started refusing the two that held a value; before it they were written silently, and on 9.24 the case list is not read at all (forced back in: 8 errors, CE0079/CE0773). Line, LineType, ReturnVariableName and the parameter keys were never refused because the floor looks a key up on the element's own $Type only and these are declared on supertypes (Flow, MicroflowBase) or under another $Type (MicroflowParameterObject). And gen bound the single case under its SDK name \"CaseValue\"; Studio Pro stores it as \"NewCaseValue\"", "file": "`mdl/backend/modelsdk/microflow_write.go` (flowVersion, microflowEncoder, sequenceFlowToGen, annotationFlowToGen, microflowParameterToGen), `nanoflow_write.go`, `rule_write.go`, `microflow_mutator_write.go`, `microflow_readback.go`, `microflow.go` (flowsFromGen reads the bezier vectors), `modelsdk/gen/microflows/types.go` (initSequenceFlow storage-name override), `modelsdk/codec/encoder.go` (OmitKeys applies to registered defaults)", "insight": "Get the old shape from a real document, not the gen: dump a Studio Pro-authored 9.x flow (any marketplace module's microflow with a decision) and run `mx convert -p` to a mid-10 version (10.12.19 here) to get a Studio Pro-serialized document for that version for free — its key sets settle LineType (10.8-10.15 only), NewCaseValue, StableId and the absent parameter keys in one dump, and mxcli's output can be diffed key-set for key-set against it. The floor (canon.StripUndeclaredProperties) is a backstop, not a gate: it cannot see supertype-declared keys, so a writer clean under the floor can still write keys the version lacks — check the Model SDK's versionInfo for the type AND its supertypes. A refusal at exec time leaves earlier statements written; #1434 (check is not version-aware) is the same root cause and is cured by the writer emitting a storable shape, not by teaching check the floor."} diff --git a/.claude/skills/fix-issue/findings/mdl-backend/2026-10-12-system-java-action-by-name.json b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-12-system-java-action-by-name.json new file mode 100644 index 0000000000..e4a8152627 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-12-system-java-action-by-name.json @@ -0,0 +1 @@ +{"area": "mdl/backend", "date": "2026-10-12", "refs": ["mendixlabs/mxcli#1445"], "symptom": "`list java actions in System` showed System.VerifyPassword, but `describe java action System.VerifyPassword` failed with \"java action not found: System.VerifyPassword\", and exec of a microflow calling it logged \"warning: could not look up Java action System.VerifyPassword: java action not found ... (entity type params will be empty)\"", "cause": "ReadJavaActionByName only scanned stored JavaAction units, after first resolving the module by name, which fails for System since it has no stored module unit; the System built-ins are virtual (meta.SystemJavaActions) and were synthesized on the list paths only", "file": "`mdl/backend/modelsdk/java_read.go` (ReadJavaActionByName, systemJavaActionByName)", "insight": "Virtual System-module members have to be synthesized on EVERY read path of a doctype, not just the list path: when one is fixed, grep the backend for the matching by-name reader. The fallback must also sit before the module lookup, because GetModuleByName(\"System\") returns nothing. Stored units still win, and an unknown System.X stays 'not found' (the test's control)."} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-alter-settings-javaversion-noop-on-9x.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-alter-settings-javaversion-noop-on-9x.json new file mode 100644 index 0000000000..1a745e6559 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-alter-settings-javaversion-noop-on-9x.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-12", "symptom": "`alter settings runtime ( JavaVersion: '17' )` on a Mendix 9.24 model printed \"Unchanged model settings\" — a silent no-op instead of an error that the property does not exist on that version", "cause": "`refuseIfNotStored` exempted JavaVersion unconditionally because it is stored under one of two keys (JavaVersion / JavaMajorVersion), but a 9.x document carries neither, and `settingsoverlay.SetJavaVersion` writes nothing when `JavaVersionKey(raw)==\"\"`", "file": "`mdl/executor/cmd_settings.go` (`refuseIfNotStored`)", "insight": "An exemption from a presence gate written for 'stored under an alias' must still check that some alias is present: the alias resolver returning \"\" is the not-stored case. DESCRIBE already gated on `JavaVersionKey != \"\"`; the ALTER path had not been made symmetric. Revert proof: MockBackend with a 9.24-shaped part (no Java key) returns nil from alterSettings", "refs": ["mendixlabs/mxcli#1438"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-child-slot-content-described-not-template.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-child-slot-content-described-not-template.json new file mode 100644 index 0000000000..d86da20d4a --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-child-slot-content-described-not-template.json @@ -0,0 +1 @@ +{"date": "2026-10-12", "area": "mdl/executor", "symptom": "mendixlabs/mxcli#1400: describe emits `content { }` for native pluggable-widget child slots (Repeater, SafeAreaView); check fails \"`content` is not a container of `repeater` — it declares: template [MDL-WIDGET26]\" and exec fails \"content needs a name\". 35 occurrences in the round-trip runs.", "cause": "Two derivations of one keyword. The widget definition names a widgets-typed slot with mdlContainerForWidgetSlot (content -> TEMPLATE, plus per-widget overrides); describe's extractChildSlots borrowed deriveObjectListKeyword, the OBJECT-LIST convention (singularise, its own override table), which leaves `content` as is.", "file": "mdl/executor/cmd_pages_describe_childslots.go", "fix": "extractChildSlots derives the keyword with mdlContainerForWidgetSlot(widgetID, key), the function the definition side uses.", "test": "TestExtractChildSlots_KeywordMatchesDefinition (tagContentContainer slot is the control)", "insight": "**When describe and the definition each compute a keyword, they must call the same function** — describe borrowed the nearest-looking helper (object lists) and agreed with the definition on every key except the overridden ones, so only a widget with a `content` slot showed it. The MDL-WIDGET26 message lists what the definition declares; compare that against the describe output before reading either side's code. Not covered: a placeholder inside a SafeAreaView is invisible to layoutPlaceholderNames (mdl/backend/modelsdk/layout_write.go), which walks only typed containers, not a CustomWidget's raw child slots."} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-describe-java-action-param-separator-after-doc-comment.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-describe-java-action-param-separator-after-doc-comment.json new file mode 100644 index 0000000000..fdf5782747 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-describe-java-action-param-separator-after-doc-comment.json @@ -0,0 +1 @@ +{"date": "2026-10-12", "area": "mdl/executor", "symptom": "`describe java action` on an action with a documented parameter printed `Name: String not null -- This sets the Name of the tool...,` so the comma sat inside the comment and `mxcli check` of the output failed `line 255:4 mismatched input 'Name' expecting ')'`.", "cause": "The parameter loop wrote the `, ` separator at the start of the next iteration, i.e. after the previous parameter's trailing `-- doc` comment. The JavaScript action describer is a copy of the same loop and had the same bug.", "fix": "Write the separator at the end of each non-final parameter, before its `--` comment (cmd_javaactions.go and cmd_javascript_actions.go).", "insight": "Any describer that emits a trailing `--` comment must emit list separators before it; a separator written 'before the next item' is only safe when nothing line-terminating can come between. grep `\" -- \"` in mdl/executor for other list emitters. A describe->visitor.Build round trip catches it; a substring assertion on the describe text does not. Avoid naming a test parameter `Count`: it is an MDL keyword and the parser's keyword hint then masks the real error.", "issue": "mendixlabs/mxcli#1412", "file": "mdl/executor/cmd_javaactions.go, mdl/executor/cmd_javascript_actions.go (parameter loop)", "test": "mdl/executor/describe_code_action_param_separator_test.go TestDescribeJavaAction_ParamDocCommentKeepsSeparator, TestDescribeJavaScriptAction_ParamDocCommentKeepsSeparator; control: both fail `mismatched input 'Amount' expecting ')'` with the fix reverted"} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-describe-user-role-drops-check-security-false.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-describe-user-role-drops-check-security-false.json new file mode 100644 index 0000000000..4e041d03c9 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-describe-user-role-drops-check-security-false.json @@ -0,0 +1 @@ +{"date": "2026-10-12", "area": "mdl/executor", "symptom": "`describe user role` omits `CheckSecurity` when it is false; create role with CheckSecurity: false, describe, drop, replay -> `show user roles` says Check Security: Yes. Silent security change on replay.", "cause": "formatUserRoleMDL followed 'leave out a property at Mendix's default (R12)' but printed CheckSecurity only when true, the inverse of the default: Security$UserRole.CheckSecurity defaults to true (modelsdk/canon/studiopro_property_defaults.json), and genSec.NewUserRole writes it so. `create or modify` keeps an unstated stored value too, so even replay onto an existing role kept true.", "fix": "Always print `CheckSecurity: ` in formatUserRoleMDL.", "insight": "An 'omit at default' describer is only correct if the omitted value equals what the writer gives a new element; check the default in studiopro_property_defaults.json, not Go's zero value. The executor mock hid it: userRoleCtx's AddUserRole built roles with Go's zero CheckSecurity (false), matching the describer's wrong assumption, so the #707 round-trip test was green. A mock write method must mirror the real writer's defaults or the round trip proves nothing. For a security flag prefer always-explicit: create-or-modify keeps unstated values, so omission is never safe on replay onto an existing role.", "issue": "mendixlabs/mxcli#1427", "file": "mdl/executor/cmd_security.go (formatUserRoleMDL)", "test": "mdl/executor/issue707_remaining_test.go TestIssue1427_UserRoleCheckSecurityFalseSurvivesReplay; mdl-examples/bug-tests/1427-user-role-check-security-false.mdl; control: before the fix 'recreated role has CheckSecurity true, want false' and 'modified role kept CheckSecurity true, want false'"} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-image-collection-same-script-ref.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-image-collection-same-script-ref.json new file mode 100644 index 0000000000..3ddccd1c44 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-image-collection-same-script-ref.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-10-12","symptom":"a page whose `image` widget names `ModA.Icons.pix` was refused by check (\"image collection not found: ModA.Icons (referenced as ModA.Icons.pix)\") and by exec (\"Refusing to execute: 1 unresolved reference(s) above. Nothing was written.\") when the same script created the collection; split into two scripts it passed (mendixlabs/mxcli#1419)","cause":"validateWidgetReferences resolved image refs against the project only, under a comment saying MDL cannot create an image collection — stale since CreateImageCollectionStmt existed; scriptContext had no image-collection or image sets","file":"`mdl/executor/helpers.go` (validateWidgetReferences image branch), `mdl/executor/validate.go` (scriptContext.imageCollections/images, collectSingle)","insight":"**A comment justifying a missing same-script exemption ages into the bug.** Every other widget reference kind merges `sc.` into the project set; grep `validateWidgetReferences` for a kind that does not, and check whether the statement that creates it now exists. Merge only into non-nil project sets: nil means the project could not be read and must keep reporting nothing. The pre-check context is whole-script (collectDefinitions), like the other kinds — statement-order is not enforced here","refs":["mendixlabs/mxcli#1419"],"ce":[]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-import-mapping-forward-ref-null-entity.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-import-mapping-forward-ref-null-entity.json new file mode 100644 index 0000000000..719b48c7c5 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-import-mapping-forward-ref-null-entity.json @@ -0,0 +1 @@ +{"date": "2026-10-12", "area": "mdl/executor", "symptom": "mendixlabs/mxcli#1418: a script with `import from mapping X` in a microflow ABOVE `create import mapping X` passed check --references and exec ('Created microflow'), and mx check then could not load the project: InvalidOperationException setting 'Entity' of an Object type -> ArgumentNullException: Value cannot be null at Mendix.Modeler.DataTypes.EntityType.set_EntityId. Re-running the microflow once the mapping existed printed 'Unchanged microflow' and the project stayed unloadable.", "cause": "addImportFromMappingAction dropped the GetImportMappingByQualifiedName error and left ResultEntityID empty; importXmlActionToGen wrote VariableType.Entity unconditionally. MDL-ORDER01 had no import-mapping kind. On the re-run, describe prints the broken activity as exactly the statement that would repair it, so the create-or-modify statement diff found no change.", "file": "mdl/executor/cmd_microflows_builder_calls.go; mdl/executor/validate_definition_order.go; mdl/executor/helpers.go; mdl/executor/cmd_flow_modify.go; mdl/backend/modelsdk/microflow_import_entity_guard.go; mdl/backend/modelsdk/microflow_write.go; mdl/backend/modelsdk/microflow_mutator_write.go", "fix": "Builder refuses an unresolved mapping or one with no root entity; MDL-ORDER01 (both tiers) gains an 'import mapping' kind for import from mapping; Create/UpdateMicroflow and the splice SerializeObject refuse an ImportXmlAction with empty ResultEntityID (loop bodies walked); planFlowModify treats a stored import activity with no result entity as not spliceable, so mdl 0 rebuilds (heals) and mdl 1 refuses with drop-and-create advice instead of 'Unchanged'.", "test": "TestImportFromMappingRefusesUnresolvedMapping, TestValidateScriptDefinitionOrder_ImportMappingForwardRef, TestImportMappingForwardRefExecRefuses1418, TestImportMappingEmptyStoredEntityIsNotUnchanged1418, TestWriterRefusesImportMappingWithoutResultEntity; mdl-examples/bug-tests/1418-import-mapping-forward-ref.fail.mdl", "insight": "**A dropped lookup error in a flow builder is a load error waiting to happen**: the value it would have produced is usually a typed identifier (here an EntityIdentifier), and empty is not 'unset' to Mendix, it is null into a setter. grep builders for `err == nil {` around backend lookups - the missing else is the bug. **Fixing the writer does not fix projects already written**: create-or-modify compares the declared statement with describe's rendering of the stored flow, and a value describe never prints (the result entity is inferred from the mapping, not stated) cannot differ, so the heal needs an explicit stored-shape check before the diff. Controls: revert builder+writer -> pedapp exec test stores Entity \"\"; revert the flow_modify check -> re-run prints 'Unchanged microflow: MyFirstModule.MF_D'. mx 11.6.6: old binary -> ArgumentNullException at load; fixed binary -> exec refused, mx check 0 errors; fixed mdl 0 re-run over the old-corrupted copy -> 'Replaced microflow', mx check 0 errors. Not covered: REST call `returns mapping X as E` with a forward mapping still writes (it states the entity itself; only ForceSingleOccurrence is inferred from the mapping).", "refs": ["mendixlabs/mxcli#1418"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-list-sort-inherited-attribute-ce1613.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-list-sort-inherited-attribute-ce1613.json new file mode 100644 index 0000000000..f788103bc5 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-list-sort-inherited-attribute-ce1613.json @@ -0,0 +1 @@ +{"date": "2026-10-12", "area": "mdl/executor", "symptom": "mendixlabs/mxcli#1430: list-operation `$L = sort $BookChunkList by Similarity desc;` with Similarity declared on the generalization wrote `Embeddings_SemanticSearch.BookChunk.Similarity`; mxbuild CE1613 \"The selected attribute ... no longer exists.\" Describe round-trips identically, so a describe-only comparison calls it in sync.", "cause": "addListOperationAction's ListOpSort case qualified a bare attribute as listEntity + \".\" + attr. The retrieve `sort by` path had been fixed for the same class (CapTrackV2 §13) with resolveAttributeInEntityHierarchy, but the list-operation sort was a separate copy of the qualification and was not.", "file": "mdl/executor/cmd_microflows_builder_actions.go", "fix": "ListOpSort qualifies via fb.resolveAttributeInEntityHierarchy, falling back to the list entity when the model does not resolve it (unit tests without a backend, unknown attribute). FIND/FILTER already go through resolveMemberChange, which walks the hierarchy.", "test": "TestListSort_InheritedAttributeUsesTheDeclaringEntity, TestListSort_OwnAttributeIsUnchanged; mdl-examples/bug-tests/1430-list-sort-inherited-attribute.mdl", "insight": "**When a qualification bug is fixed, grep every `entityType + \".\" +` in the builder — the fix for one path does not reach its copies.** The retrieve sort, list sort and aggregate attribute each built Module.Entity.Attr independently; only the first walked the generalization chain. The aggregate `sum/min/max $L by Attr` path still prefixes blindly (same CE1613 class, not fixed here). Describe cannot detect this: it prints the bare attribute name either way, so only the stored AttributeQualifiedName (bson dump) or mxbuild shows it. The test reuses inheritedSortBackend from the §13 test; written before the fix it failed with stored [SyntheticApp.AppUser.Name]."} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-output-line-limit-silent-truncation.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-output-line-limit-silent-truncation.json new file mode 100644 index 0000000000..c0f8d817f7 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-output-line-limit-silent-truncation.json @@ -0,0 +1 @@ +{"date": "2026-10-12", "area": "mdl/executor", "symptom": "mendixlabs/mxcli#1425: `describe module ... with all` truncates at 10 000 lines without a warning — output stops in the middle of a statement, exit 0.", "cause": "outputGuard.Write returns an error once the per-statement limit (maxOutputLines, 10_000) is passed and silently discards everything after, but every describer ignores Fprintf's error, so the handler returned nil and Executor.Execute never looked at guard.exceeded.", "file": "mdl/executor/output_guard.go; mdl/executor/executor.go", "fix": "Execute: if the statement otherwise succeeded but the guard was exceeded, return the limit error (now saying the output was truncated, with a hint to describe per document). No knob raises the limit; none was added.", "test": "TestExecuteReportsOutputLineLimitExceeded (handler writing maxOutputLines lines is the control)", "insight": "**An error returned from io.Writer.Write is not a reporting channel** — nearly all output code ignores it. A guard that signals through Write must also be checked by whoever owns the statement's result. The tell for this class is an output length that is exactly a round constant: grep the number before suspecting the describer."} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-returns-as-variable-undeclared-messages.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-returns-as-variable-undeclared-messages.json new file mode 100644 index 0000000000..8c4b4d336b --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-returns-as-variable-undeclared-messages.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-12", "refs": ["mendixlabs/mxcli#1436"], "symptom": "`create or modify microflow M.F () returns Boolean as $Closed begin $Closed = true; return $Closed; end;` was refused with \"a reassignment says `set` under mdl 1: write `set $Closed = true;`\", and the `set` form then with \"variable 'Closed' is not declared\" (check -p --references) - each message pointed at the other", "cause": "Semantics were right (`returns … as $X` only names the result, Mendix's ReturnVariableName, and declares nothing); the two messages were each written for the common case: the visitor's MDL-V1 setIsMandatory error assumed the target already exists, and the flow validator's not-declared error did not know the flow's return clause", "file": "`mdl/visitor/visitor_list_activities.go` (setIsMandatory message), `mdl/executor/cmd_microflows_builder.go` (addUndeclaredSetError), `mdl/executor/cmd_microflows_builder_validate.go` (validateFlowBody now takes the return type)", "insight": "Message-only fix, no semantics change. The visitor cannot tell a reassignment from a first assignment (it has no scope), so its `set` advice must offer `declare` too. The not-declared error is only reproducible WITH a project: plain `mxcli check` passes the `set` form, because validateFlowBody runs from check --references and exec - reproduce with -p before concluding a report is wrong. validateFlowBody had no return type; flowBuilder.returnType already existed (set by buildFlowGraph on exec) so it is reused, and nothing in the validate path reads it otherwise."} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-set-on-aggregate-output-undeclared.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-set-on-aggregate-output-undeclared.json new file mode 100644 index 0000000000..4d0b347bb7 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-set-on-aggregate-output-undeclared.json @@ -0,0 +1 @@ +{"date": "2026-10-12", "area": "mdl/executor", "symptom": "mendixlabs/mxcli#1413: `$Sum = sum $List by Amount; set $Sum = $Sum + 1;` (also count) passed `check -p` but `exec` refused the microflow with \"variable 'Sum' is not declared\".", "cause": "Two passes keep their own declared-variable bookkeeping. validateStatements declared an AggregateListStmt's output as Unknown; addAggregateListAction (the build pass exec runs) never put it in declaredVars or varTypes, so addChangeVariableAction's isVariableDeclared check failed only at exec.", "file": "mdl/executor/cmd_microflows_builder_actions.go", "fix": "addAggregateListAction declares the output: count Integer, average Decimal, all/any Boolean, reduce its declared return type, sum/min/max Unknown (attribute-typed, not resolved here).", "test": "TestAggregateOutputIsDeclaredForSet; mdl-examples/bug-tests/1413-set-on-aggregate-output.mdl", "insight": "**check passing while exec refuses is a validate-vs-build bookkeeping split**: diff the statement's case in cmd_microflows_builder_validate.go against its add*Action builder — whichever registers declaredVars/varTypes where the other does not is the bug. A builder unit test driving buildFlowGraph with the producing statement followed by a SET reproduces it with no project, and the control (fix reverted) fails with the exact reported message. Worth checking the same shape in other output-producing builders: addListOperationAction registers only list/object outputs in varTypes, so a Boolean CONTAINS/EQUALS output is likewise undeclared in the build pass."} diff --git a/.claude/skills/fix-issue/findings/mdl-visitor/2026-10-12-odata-expose-quoted-member-name.json b/.claude/skills/fix-issue/findings/mdl-visitor/2026-10-12-odata-expose-quoted-member-name.json new file mode 100644 index 0000000000..8f14662404 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-visitor/2026-10-12-odata-expose-quoted-member-name.json @@ -0,0 +1 @@ +{"area":"mdl/visitor","date":"2026-10-12","symptom":"published OData `expose (Name as 'Name' (KEY), \"Description\")` stored the member name with its double quotes; check and exec were clean, describe printed `\"Description\" as '\"Description\"'`, mx check reported CE1613 \"The selected attribute 'ModA.EntA.\"Description\"' no longer exists\" (mendixlabs/mxcli#1423). Published microflow parameters had the same defect","cause":"parseExposeMembers and the publish-microflow expose loop took `member.IdentifierOrKeyword().GetText()`, which keeps a QUOTED_IDENTIFIER's quotes; every other identifierOrKeyword slot in the file already used identifierOrKeywordText","file":"`mdl/visitor/visitor_odata.go` (parseExposeMembers, publish microflow expose)","insight":"**`identifierOrKeyword().GetText()` is the raw token — quotes included.** Any name slot reading it bypasses the quoting the skills recommend, and nothing downstream notices because the quoted string is still a valid-looking name until mx resolves it. `grep -n 'IdentifierOrKeyword().GetText()' mdl/visitor` is the audit; the remaining hits are enum/keyword values or already wrapped in unquoteIdentifier. Describe round-trip is the cheapest repro: the stored quotes reappear inside the `as '...'` exposed name","refs":["mendixlabs/mxcli#1423"],"ce":["CE1613"]} diff --git a/cmd/mxcli/check_multi_file_summary_test.go b/cmd/mxcli/check_multi_file_summary_test.go new file mode 100644 index 0000000000..7904a4e838 --- /dev/null +++ b/cmd/mxcli/check_multi_file_summary_test.go @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "strings" + "testing" +) + +// A check of several files ends on a verdict for the whole set +// (mendixlabs/mxcli#1431). It used to end on the last file's own +// "Check passed!", so a failing file followed by a passing one read as a pass +// to anything looking at the tail of the log; only the exit code said 1. +func TestCheck_ScriptSetEndsOnOverallVerdict(t *testing.T) { + if f := checkCmd.Flags().Lookup("format"); f != nil { + _ = f.Value.Set(f.DefValue) + } + scripts := t.TempDir() + bad := writeScript(t, scripts, "bad.mdl", "create persistent entity M.E ( Name: String(100) \n") + good := writeScript(t, scripts, "good.mdl", "create persistent entity M.F ( Name: String(100) );\n") + + lastLine := func(out string) string { + lines := strings.Split(strings.TrimRight(out, "\n"), "\n") + return strings.TrimSpace(lines[len(lines)-1]) + } + + var code int + out := captureStd(t, func() { code = runCheckFiles(checkCmd, []string{bad, good}) }) + if code == 0 { + t.Fatalf("a set with a failing file exited 0:\n%s", out) + } + if want := "1 of 2 files failed: " + bad; lastLine(out) != want { + t.Errorf("last line = %q, want %q\n%s", lastLine(out), want, out) + } + + // Control: a set that passes says so for the whole set. + out = captureStd(t, func() { code = runCheckFiles(checkCmd, []string{good, good}) }) + if code != 0 { + t.Fatalf("a passing set exited %d:\n%s", code, out) + } + if want := "All 2 files passed"; lastLine(out) != want { + t.Errorf("last line = %q, want %q\n%s", lastLine(out), want, out) + } +} diff --git a/cmd/mxcli/cmd_check.go b/cmd/mxcli/cmd_check.go index e1d45cf446..f161746b8a 100644 --- a/cmd/mxcli/cmd_check.go +++ b/cmd/mxcli/cmd_check.go @@ -577,7 +577,7 @@ func runCheckFiles(cmd *cobra.Command, files []string) int { scratch = sc } worst := 0 - var notApplied []string + var notApplied, failed []string for i, f := range files { if i > 0 { fmt.Println() @@ -596,12 +596,24 @@ func runCheckFiles(cmd *cobra.Command, files []string) int { if code > worst { worst = code } + if code != 0 { + failed = append(failed, f) + } if scratch != nil && i < len(files)-1 { if why := applyToScratch(scratch, f, code); why != "" { notApplied = append(notApplied, why) } } } + // Each file ends on its own verdict, so without this the run's last line + // was the last file's "Check passed!" even after an earlier file failed + // (mendixlabs/mxcli#1431). Close on the verdict for the whole set. + fmt.Println() + if len(failed) > 0 { + fmt.Printf("%d of %d files failed: %s\n", len(failed), len(files), strings.Join(failed, ", ")) + } else { + fmt.Printf("All %d files passed\n", len(files)) + } return worst } diff --git a/cmd/mxcli/cmd_describe.go b/cmd/mxcli/cmd_describe.go index 70cde93b92..2242c7d3d7 100644 --- a/cmd/mxcli/cmd_describe.go +++ b/cmd/mxcli/cmd_describe.go @@ -31,7 +31,7 @@ can match both an entity and a document, for example) or for forms that have no single qualified name (module, settings, navigation, module role). Types: - module Describe a module (all contents) + module Describe a module and its module roles (--all: everything in it) entity Describe an entity externalentity Describe an external entity (alias for entity) association Describe an association @@ -39,6 +39,7 @@ Types: constant Describe a constant microflow Describe a microflow nanoflow Describe a nanoflow + rule Describe a rule workflow Describe a workflow page Describe a page snippet Describe a snippet @@ -78,6 +79,7 @@ Example: mxcli describe -p app.mpr MyModule.Customer # type auto-detected mxcli describe -p app.mpr MyModule.ProcessOrder # type auto-detected mxcli describe -p app.mpr module MyModule + mxcli describe -p app.mpr --all module MyModule # DESCRIBE MODULE ... WITH ALL mxcli describe -p app.mpr entity MyModule.Customer mxcli describe -p app.mpr microflow MyModule.ProcessOrder mxcli describe -p app.mpr --mdl 0 microflow MyModule.ProcessOrder @@ -133,10 +135,16 @@ Example: mdlCmd, ok := describeMDLCommand(objectType, name) if !ok { fmt.Fprintf(os.Stderr, "Unknown type: %s\n", strings.Join(args[:len(args)-1], " ")) - fmt.Fprintln(os.Stderr, "Valid types: module, entity, association, enumeration, constant, microflow, nanoflow, workflow, page, snippet, layout, javaaction, javascriptaction, jsonstructure, importmapping, exportmapping, consumedrestservice, consumedodataservice, publishedodataservice, publishedrestservice, imagecollection, menu, taskqueue, scheduledevent, businesseventservice, databaseconnection, agent, aimodel, knowledgebase, consumedmcpservice, datatransformer, modulerole, userrole, appsecurity, settings, demouser, navigation, systemoverview") + fmt.Fprintln(os.Stderr, "Valid types: module, entity, association, enumeration, constant, microflow, nanoflow, rule, workflow, page, snippet, layout, javaaction, javascriptaction, jsonstructure, importmapping, exportmapping, consumedrestservice, consumedodataservice, publishedodataservice, publishedrestservice, imagecollection, menu, taskqueue, scheduledevent, businesseventservice, databaseconnection, agent, aimodel, knowledgebase, consumedmcpservice, datatransformer, modulerole, userrole, appsecurity, settings, demouser, navigation, systemoverview") fmt.Fprintln(os.Stderr, "Multi-word types also accepted: json structure, import mapping, export mapping, consumed rest service, consumed odata service, published odata service, published rest service, task queue, app security, image collection, scheduled event, regular expression, business event service, ai model, knowledge base, consumed mcp service, data transformer, etc.") os.Exit(1) } + all, _ := cmd.Flags().GetBool("all") + mdlCmd, err := applyDescribeAll(objectType, mdlCmd, all) + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + os.Exit(1) + } exec, logger := newLoggedExecutor("subcommand") defer logger.Close() @@ -257,6 +265,8 @@ func describeMDLCommand(objectType, name string) (mdlCmd string, ok bool) { return fmt.Sprintf("DESCRIBE MICROFLOW %s", name), true case "NANOFLOW": return fmt.Sprintf("DESCRIBE NANOFLOW %s", name), true + case "RULE": + return fmt.Sprintf("DESCRIBE RULE %s", name), true case "WORKFLOW": return fmt.Sprintf("DESCRIBE WORKFLOW %s", name), true case "PAGE": @@ -339,6 +349,20 @@ func describeMDLCommand(objectType, name string) (mdlCmd string, ok bool) { return "", false } +// applyDescribeAll applies the --all flag: for a module it describes everything +// in it (DESCRIBE MODULE ... WITH ALL), where a plain describe module prints the +// module and its roles only (mendixlabs/mxcli#1435). --all on any other type is +// an error rather than a flag silently ignored. +func applyDescribeAll(objectType, mdlCmd string, all bool) (string, error) { + if !all { + return mdlCmd, nil + } + if objectType != "MODULE" { + return "", fmt.Errorf("--all applies to describe module only") + } + return mdlCmd + " WITH ALL", nil +} + // objectTypeToDescribe maps a catalog `objects` view ObjectType to the `describe` // keyword. Only directly describable types with a `Module.Name` shape are listed. var objectTypeToDescribe = map[string]string{ @@ -348,6 +372,7 @@ var objectTypeToDescribe = map[string]string{ "ASSOCIATION": "association", "MICROFLOW": "microflow", "NANOFLOW": "nanoflow", + "RULE": "rule", "PAGE": "page", "SNIPPET": "snippet", "LAYOUT": "layout", @@ -382,11 +407,11 @@ var objectTypeToDescribe = map[string]string{ // unitTypeToDescribe maps a top-level unit's BSON $Type to the `describe` // keyword. Used by the live-reader fallback. Only directly describable document -// types are listed (page templates, building blocks, rules, etc. are absent so -// they don't resolve). +// types are listed (page templates etc. are absent so they don't resolve). var unitTypeToDescribe = map[string]string{ "Microflows$Microflow": "microflow", "Microflows$Nanoflow": "nanoflow", + "Microflows$Rule": "rule", "Forms$Page": "page", "Forms$Snippet": "snippet", "Pages$BuildingBlock": "buildingblock", diff --git a/cmd/mxcli/cmd_describe_test.go b/cmd/mxcli/cmd_describe_test.go index 67131716a3..94eb52060e 100644 --- a/cmd/mxcli/cmd_describe_test.go +++ b/cmd/mxcli/cmd_describe_test.go @@ -187,3 +187,69 @@ func TestDescribeHelp_StudioProDocumentNames(t *testing.T) { t.Errorf("unitTypeToDescribe[Queues$Queue] = %q, want taskqueue", got) } } + +// TestDescribeModule_HelpMatchesOutput is mendixlabs/mxcli#1435: the help said +// "module Describe a module (all contents)" while the subcommand ran a plain +// DESCRIBE MODULE, which prints only the module and its roles. The contents are +// opt-in with --all (DESCRIBE MODULE ... WITH ALL), and the help says so. +func TestDescribeModule_HelpMatchesOutput(t *testing.T) { + if strings.Contains(describeCmd.Long, "Describe a module (all contents)") { + t.Error(`describe --help promises "all contents" for a plain describe module`) + } + if describeCmd.Flags().Lookup("all") == nil { + t.Fatal("describe has no --all flag") + } + + plain, _ := describeMDLCommand("MODULE", "M") + for _, tc := range []struct { + all bool + want string + }{ + {false, "DESCRIBE MODULE M"}, + {true, "DESCRIBE MODULE M WITH ALL"}, + } { + got, err := applyDescribeAll("MODULE", plain, tc.all) + if err != nil || got != tc.want { + t.Errorf("describe module M (all=%v) = %q, %v; want %q", tc.all, got, err, tc.want) + } + if _, errs := visitor.Build(got); len(errs) > 0 { + t.Errorf("%q does not parse: %v", got, errs) + } + } + + // --all on any other type is refused rather than silently ignored. + entity, _ := describeMDLCommand("ENTITY", "M.E") + if _, err := applyDescribeAll("ENTITY", entity, true); err == nil { + t.Error("describe --all entity M.E: want an error, got none") + } + if got, err := applyDescribeAll("ENTITY", entity, false); err != nil || got != entity { + t.Errorf("describe entity M.E without --all = %q, %v; want %q", got, err, entity) + } +} + +// TestDescribeMDLCommand_Rule is mendixlabs/mxcli#1402: `describe rule M.R` +// answered "Unknown type: rule" although `-c "describe rule M.R"` works, and a +// bare `describe M.R` found "no describable document" because neither +// auto-detect map knew rules. +func TestDescribeMDLCommand_Rule(t *testing.T) { + const want = "DESCRIBE RULE M.R" + got, ok := describeMDLCommand("RULE", "M.R") + if !ok { + t.Fatal("describe rule M.R: Unknown type: rule") + } + if got != want { + t.Errorf("describe rule M.R = %q, want %q", got, want) + } + if _, errs := visitor.Build(want); len(errs) > 0 { + t.Fatalf("%q does not parse: %v", want, errs) + } + if got := objectTypeToDescribe["RULE"]; got != "rule" { + t.Errorf("catalog ObjectType RULE auto-detects as %q, want rule", got) + } + if got := unitTypeToDescribe["Microflows$Rule"]; got != "rule" { + t.Errorf("unit type Microflows$Rule auto-detects as %q, want rule", got) + } + if !strings.Contains(describeCmd.Long, "\n rule ") { + t.Error("describe --help lists no rule type line") + } +} diff --git a/cmd/mxcli/cmd_marketplace.go b/cmd/mxcli/cmd_marketplace.go index f6f5c3bf38..3a9cb256c8 100644 --- a/cmd/mxcli/cmd_marketplace.go +++ b/cmd/mxcli/cmd_marketplace.go @@ -72,10 +72,12 @@ var marketplaceDownloadCmd = &cobra.Command{ By default the latest version is downloaded. Use --version to pick a specific version number. The file is written to the current directory under its CDN -filename unless -o is given.`, +filename unless -o is given; -o takes a file path, or a directory to write the +CDN filename into.`, Example: ` mxcli marketplace download 2888 mxcli marketplace download 2888 --version 7.0.2 - mxcli marketplace download 2888 -o ./mods/dbc.mpk`, + mxcli marketplace download 2888 -o ./mods/dbc.mpk + mxcli marketplace download 2888 -o ./mods/`, Args: cobra.ExactArgs(1), RunE: runMarketplaceDownload, } @@ -95,7 +97,7 @@ func init() { marketplaceDownloadCmd.Flags().String("profile", auth.ProfileDefault, "credential profile") marketplaceDownloadCmd.Flags().String("version", "", "version number to download (default: latest)") - marketplaceDownloadCmd.Flags().StringP("output", "o", "", "output path (default: CDN filename in the current directory)") + marketplaceDownloadCmd.Flags().StringP("output", "o", "", "output file or directory (default: CDN filename in the current directory)") marketplaceCmd.AddCommand(marketplaceSearchCmd) marketplaceCmd.AddCommand(marketplaceInfoCmd) @@ -150,9 +152,26 @@ func runMarketplaceDownload(cmd *cobra.Command, args []string) error { // Stream to a temp file first, then derive the final name and rename, so a // failed/cancelled download never leaves a truncated .mpk in place. + // + // An --output naming a directory (an existing one, or any path ending in a + // separator) receives the package under the name it would otherwise get; + // renaming the temp file onto the directory itself failed with "file + // exists" (mendixlabs/mxcli#1443). dir := "." + intoDir := false if output != "" { dir = filepath.Dir(output) + if strings.HasSuffix(output, "/") || strings.HasSuffix(output, string(os.PathSeparator)) { + intoDir = true + } else if fi, err := os.Stat(output); err == nil && fi.IsDir() { + intoDir = true + } + if intoDir { + if err := os.MkdirAll(output, 0o755); err != nil { + return fmt.Errorf("create output directory: %w", err) + } + dir = output + } } tmp, err := os.CreateTemp(dir, ".mxcli-download-*.part") if err != nil { @@ -171,11 +190,14 @@ func runMarketplaceDownload(cmd *cobra.Command, args []string) error { } dest := output - if dest == "" { + if dest == "" || intoDir { if filename == "" { filename = fmt.Sprintf("content-%d-%s.mpk", contentID, version.VersionNumber) } dest = filename + if intoDir { + dest = filepath.Join(output, filename) + } } if err := os.Rename(tmpName, dest); err != nil { return fmt.Errorf("write %s: %w", dest, err) diff --git a/cmd/mxcli/cmd_marketplace_download_test.go b/cmd/mxcli/cmd_marketplace_download_test.go index 1411bff0bd..aedb0a201d 100644 --- a/cmd/mxcli/cmd_marketplace_download_test.go +++ b/cmd/mxcli/cmd_marketplace_download_test.go @@ -3,128 +3,86 @@ package main import ( - "bytes" "fmt" "net/http" "os" "path/filepath" "strings" "testing" - - "github.com/mendixlabs/mxcli/internal/marketplace" ) -func TestStartSearchProgress_Gating(t *testing.T) { - t.Run("nil for --json", func(t *testing.T) { - c := marketplace.New(http.DefaultClient) - if startSearchProgress(c, os.Stderr, true) != nil { - t.Error("expected no progress for --json output") - } - if c.OnProgress != nil { - t.Error("OnProgress must stay unset for --json") - } - }) - t.Run("nil for non-terminal writer", func(t *testing.T) { - c := marketplace.New(http.DefaultClient) - // A bytes.Buffer is not an *os.File, so it's never an interactive terminal. - if startSearchProgress(c, &bytes.Buffer{}, false) != nil { - t.Error("expected no progress for a non-terminal writer (pipe/buffer)") - } - if c.OnProgress != nil { - t.Error("OnProgress must stay unset for a non-terminal writer") - } - }) -} - -func TestResolveVersion(t *testing.T) { - versions := []marketplace.Version{ - {VersionNumber: "7.0.3", DownloadURL: "u3"}, - {VersionNumber: "7.0.2", DownloadURL: "u2"}, - } - t.Run("latest (empty) picks first", func(t *testing.T) { - v, err := resolveVersion(versions, "") - if err != nil || v.VersionNumber != "7.0.3" { - t.Fatalf("got %v, %v; want 7.0.3", v, err) - } - }) - t.Run("specific match", func(t *testing.T) { - v, err := resolveVersion(versions, "7.0.2") - if err != nil || v.VersionNumber != "7.0.2" { - t.Fatalf("got %v, %v; want 7.0.2", v, err) - } - }) - t.Run("not found", func(t *testing.T) { - if _, err := resolveVersion(versions, "9.9.9"); err == nil { - t.Fatal("expected error for missing version") - } - }) - t.Run("empty list", func(t *testing.T) { - if _, err := resolveVersion(nil, ""); err == nil { - t.Fatal("expected error for empty version list") - } - }) -} - -// marketplaceTestHandler serves the content/versions/download/CDN endpoints for a -// single content item, building absolute URLs from the request host so the -// download redirect points back at the same test server. -func marketplaceTestHandler(contentType, filename, mpkBody string) http.HandlerFunc { +// downloadHandler serves a one-version list whose download resolves (303) to +// a CDN path ending in cdnName, the two hops Client.Download makes. +func downloadHandler(t *testing.T, cdnName, body string) http.HandlerFunc { + t.Helper() return func(w http.ResponseWriter, r *http.Request) { base := "http://" + r.Host switch { - case strings.HasSuffix(r.URL.Path, "/versions"): - fmt.Fprintf(w, `{"items":[{"name":"X","versionId":"v1","versionNumber":"1.0.0","minSupportedMendixVersion":"10.0.0","publicationDate":"2026-01-01T00:00:00Z","downloadUrl":%q}]}`, base+"/v1/versions/v1/download") - case strings.HasSuffix(r.URL.Path, "/download"): - http.Redirect(w, r, base+"/cdn/"+filename, http.StatusSeeOther) - case strings.HasPrefix(r.URL.Path, "/cdn/"): - _, _ = w.Write([]byte(mpkBody)) - case strings.HasPrefix(r.URL.Path, "/v1/content/"): - fmt.Fprintf(w, `{"contentId":42,"publisher":"Mendix","type":%q,"isPrivate":false}`, contentType) + case strings.HasPrefix(r.URL.Path, "/v1/content/170/versions"): + fmt.Fprintf(w, `{"items":[{"name":"Community Commons","versionNumber":"11.5.0","downloadUrl":%q}]}`, base+"/dl/170") + case r.URL.Path == "/dl/170": + http.Redirect(w, r, base+"/cdn/"+cdnName, http.StatusSeeOther) + case r.URL.Path == "/cdn/"+cdnName: + _, _ = w.Write([]byte(body)) default: - w.WriteHeader(http.StatusNotFound) + t.Errorf("unexpected request %s", r.URL) + http.NotFound(w, r) } } } -func TestMarketplaceDownload_WritesFile(t *testing.T) { - dest := filepath.Join(t.TempDir(), "out.mpk") - out, err := runMarketplace(t, marketplaceTestHandler("Module", "DatabaseConnector-v1.0.0.mpk", "PK\x03\x04mpk"), - "download", "42", "-o", dest) - if err != nil { - t.Fatalf("download failed: %v\n%s", err, out) - } - got, err := os.ReadFile(dest) +// `--output ` writes the package into that directory under its +// CDN filename (mendixlabs/mxcli#1443). It used to stream to a temp file +// beside the directory and rename it onto the directory: "rename … file exists". +func TestMarketplaceDownload_OutputDirectory(t *testing.T) { + dir := t.TempDir() + out, err := runMarketplace(t, downloadHandler(t, "CommunityCommons.mpk", "mpk-bytes"), + "download", "170", "--output", dir) if err != nil { - t.Fatalf("output not written: %v", err) + t.Fatalf("download into an existing directory: %v\n%s", err, out) } - if string(got) != "PK\x03\x04mpk" { - t.Errorf("content = %q, want the mpk body", string(got)) + want := filepath.Join(dir, "CommunityCommons.mpk") + got, rerr := os.ReadFile(want) + if rerr != nil || string(got) != "mpk-bytes" { + t.Fatalf("want the package at %s, read %q (%v)\n%s", want, got, rerr, out) } - // No temp .part file left behind in the dest directory. - entries, _ := os.ReadDir(filepath.Dir(dest)) - for _, e := range entries { - if strings.HasSuffix(e.Name(), ".part") { - t.Errorf("leftover temp file: %s", e.Name()) - } + if !strings.Contains(out, want) { + t.Errorf("output does not name %s:\n%s", want, out) } + assertNoPartFiles(t, dir) + assertNoPartFiles(t, filepath.Dir(dir)) } -func TestMarketplaceInstall_WidgetRouting(t *testing.T) { - projDir := t.TempDir() - mpr := filepath.Join(projDir, "app.mpr") - if err := os.WriteFile(mpr, []byte("x"), 0o644); err != nil { - t.Fatal(err) - } - out, err := runMarketplace(t, marketplaceTestHandler("Widget", "RadioButtonList.mpk", "PK\x03\x04widget"), - "install", "42", "-p", mpr) +// A trailing separator names a directory even when it does not exist yet. +func TestMarketplaceDownload_OutputTrailingSeparator(t *testing.T) { + dir := filepath.Join(t.TempDir(), "mods") + string(os.PathSeparator) + out, err := runMarketplace(t, downloadHandler(t, "CommunityCommons.mpk", "mpk-bytes"), + "download", "170", "-o", dir) if err != nil { - t.Fatalf("install failed: %v\n%s", err, out) + t.Fatalf("download into a new directory: %v\n%s", err, out) } - got, err := os.ReadFile(filepath.Join(projDir, "widgets", "RadioButtonList.mpk")) + if _, err := os.Stat(filepath.Join(dir, "CommunityCommons.mpk")); err != nil { + t.Fatalf("package not written into %s: %v\n%s", dir, err, out) + } +} + +// Control: a file path keeps working as before. +func TestMarketplaceDownload_OutputFile(t *testing.T) { + dest := filepath.Join(t.TempDir(), "dbc.mpk") + out, err := runMarketplace(t, downloadHandler(t, "CommunityCommons.mpk", "mpk-bytes"), + "download", "170", "-o", dest) if err != nil { - t.Fatalf("widget not placed in widgets/: %v\noutput: %s", err, out) + t.Fatalf("download to a file path: %v\n%s", err, out) } - if string(got) != "PK\x03\x04widget" { - t.Errorf("widget content = %q", string(got)) + if got, err := os.ReadFile(dest); err != nil || string(got) != "mpk-bytes" { + t.Fatalf("want the package at %s, read %q (%v)", dest, got, err) + } +} + +func assertNoPartFiles(t *testing.T, dir string) { + t.Helper() + parts, _ := filepath.Glob(filepath.Join(dir, ".mxcli-download-*.part")) + if len(parts) > 0 { + t.Errorf("temp files left behind in %s: %v", dir, parts) } } diff --git a/cmd/mxcli/cmd_marketplace_install.go b/cmd/mxcli/cmd_marketplace_install.go index c675e14fba..f7bd252573 100644 --- a/cmd/mxcli/cmd_marketplace_install.go +++ b/cmd/mxcli/cmd_marketplace_install.go @@ -11,6 +11,7 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "strings" modelsdk "github.com/mendixlabs/mxcli" @@ -32,9 +33,12 @@ Install is type-aware: - Module copied in with mxcli's own writer (new modules only) - other types downloaded to disk with import instructions -Updating a module that is already present is NOT done automatically: it could +Updating a module that is already present is NOT done by install: it could discard local edits and, for modules with persistent entities, change entity -IDs (which loses data). Such updates are reported and left to Studio Pro. +IDs (which loses data). Install reports the module and names the command that +updates it — 'marketplace diff' to see what an update would touch, then +'marketplace update -p --to ', which carries +the entity identity and role grants across. A module is installed by copying its units with mxcli's own writer rather than by running 'mx module-import'. That preserves the project's storage format — @@ -142,7 +146,7 @@ func runMarketplaceInstall(cmd *cobra.Command, args []string) error { return err } } - return installModule(cmd.Context(), client, version, mprPath, allowFormatChange, out) + return installModule(cmd.Context(), client, contentID, version, mprPath, allowFormatChange, out) default: // Theme / Starter App / Sample / unknown: download + instruct rather // than guess a placement we can't verify. @@ -170,7 +174,7 @@ func installFromFile(ctx context.Context, mpkPath, mprPath string, allowFormatCh } switch { case pkg.ModelerProject.Module.Name != "": - return installModuleFromFile(ctx, mpkPath, mprPath, allowFormatChange, "", "", + return installModuleFromFile(ctx, mpkPath, mprPath, allowFormatChange, 0, "", "", "from "+filepath.Base(mpkPath), out) case pkg.ClientModule.Name != "": return placeWidgetFile(mpkPath, filepath.Dir(mprPath), out) @@ -221,7 +225,7 @@ func printWidgetNext(out io.Writer) { // installModule imports a module .mpk into the project, but only when the module // is not already present. An existing module is reported, not modified. -func installModule(ctx context.Context, client *marketplace.Client, v *marketplace.Version, mprPath string, allowFormatChange bool, out io.Writer) error { +func installModule(ctx context.Context, client *marketplace.Client, contentID int, v *marketplace.Version, mprPath string, allowFormatChange bool, out io.Writer) error { // Download to a temp .mpk so we can inspect it and hand it to mx. tmpDir, err := os.MkdirTemp("", "mxcli-install-") if err != nil { @@ -232,7 +236,7 @@ func installModule(ctx context.Context, client *marketplace.Client, v *marketpla if err != nil { return err } - return installModuleFromFile(ctx, mpkPath, mprPath, allowFormatChange, v.VersionNumber, v.VersionID, + return installModuleFromFile(ctx, mpkPath, mprPath, allowFormatChange, contentID, v.VersionNumber, v.VersionID, "version "+v.VersionNumber, out) } @@ -240,9 +244,10 @@ func installModule(ctx context.Context, client *marketplace.Client, v *marketpla // package is on disk — shared by the online path (after its download) and by // --file (which has no download). versionNumber and versionID are the // marketplace identity to record on the module; both are empty for a package -// from disk, which has none. label is how the package is named in output. +// from disk, which has none, and so is contentID (0). label is how the package +// is named in output. func installModuleFromFile(ctx context.Context, mpkPath, mprPath string, allowFormatChange bool, - versionNumber, versionID, label string, out io.Writer) error { + contentID int, versionNumber, versionID, label string, out io.Writer) error { moduleName, err := moduleNameFromMpk(mpkPath) if err != nil { @@ -259,11 +264,8 @@ func installModuleFromFile(ctx context.Context, mpkPath, mprPath string, allowFo _ = reader.Disconnect() if existing { - // Postponed: do NOT auto-update modules — see the module-update memory. - fmt.Fprintf(out, "Module %q is already installed (version %s).\n", moduleName, displayVer(installedVer)) - fmt.Fprintf(out, "Target: %s.\n", label) - fmt.Fprintln(out, "In-place module updates are not applied automatically (they can discard local") - fmt.Fprintln(out, "edits and change persistent-entity IDs, which loses data). Update via Studio Pro.") + // Install never updates implicitly: an update is its own command. + reportModuleAlreadyInstalled(out, moduleName, installedVer, label, contentID, versionNumber, mprPath) return nil } @@ -492,3 +494,25 @@ func moduleNameFromMpk(mpkPath string) (string, error) { } return "", fmt.Errorf("package.xml has no module name") } + +// reportModuleAlreadyInstalled says why install left a present module alone and +// names the command that updates it (mendixlabs/mxcli#1442). It used to send +// the user to Studio Pro, although 'marketplace update' does the update +// headlessly and carries the GUIDs and role grants a plain replace would lose. +// contentID and versionNumber are 0/"" for a --file install, which has neither; +// the command is then named with placeholders. +func reportModuleAlreadyInstalled(out io.Writer, moduleName, installedVer, label string, contentID int, versionNumber, mprPath string) { + id, to := "", "" + if contentID > 0 { + id = strconv.Itoa(contentID) + } + if versionNumber != "" { + to = versionNumber + } + fmt.Fprintf(out, "Module %q is already installed (version %s).\n", moduleName, displayVer(installedVer)) + fmt.Fprintf(out, "Target: %s.\n", label) + fmt.Fprintln(out, "It is not updated by install (an update can discard local edits). To update it,") + fmt.Fprintln(out, "review what would change, then apply it with entity identity and role grants preserved:") + fmt.Fprintf(out, " mxcli marketplace diff %s -p %s --to %s\n", id, mprPath, to) + fmt.Fprintf(out, " mxcli marketplace update %s -p %s --to %s\n", id, mprPath, to) +} diff --git a/cmd/mxcli/cmd_marketplace_install_update_hint_test.go b/cmd/mxcli/cmd_marketplace_install_update_hint_test.go new file mode 100644 index 0000000000..a26f9c1d65 --- /dev/null +++ b/cmd/mxcli/cmd_marketplace_install_update_hint_test.go @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "bytes" + "strings" + "testing" +) + +// An install of a module that is already present still refuses to update it, +// but names the command that does (mendixlabs/mxcli#1442) rather than handing +// the user off to Studio Pro, which a headless environment does not have. +func TestReportModuleAlreadyInstalled_PointsToMarketplaceUpdate(t *testing.T) { + var out bytes.Buffer + reportModuleAlreadyInstalled(&out, "Administration", "4.3.2", "version 4.5.0", 23513, "4.5.0", "X/X.mpr") + got := out.String() + for _, want := range []string{ + `Module "Administration" is already installed (version 4.3.2).`, + "not updated by install", + "mxcli marketplace diff 23513 -p X/X.mpr --to 4.5.0", + "mxcli marketplace update 23513 -p X/X.mpr --to 4.5.0", + } { + if !strings.Contains(got, want) { + t.Errorf("missing %q in:\n%s", want, got) + } + } + if strings.Contains(got, "Update via Studio Pro") { + t.Errorf("still hands off to Studio Pro:\n%s", got) + } + + // --file has no content id or marketplace version: the command is named + // with placeholders instead of invented values. + out.Reset() + reportModuleAlreadyInstalled(&out, "CompanyTheme", "", "from CompanyTheme.mpk", 0, "", "app.mpr") + got = out.String() + if !strings.Contains(got, "mxcli marketplace update -p app.mpr --to ") { + t.Errorf("--file install: no update command named:\n%s", got) + } +} diff --git a/cmd/mxcli/docker/detect.go b/cmd/mxcli/docker/detect.go index ce2654f3cf..6a8ed7f6a7 100644 --- a/cmd/mxcli/docker/detect.go +++ b/cmd/mxcli/docker/detect.go @@ -406,10 +406,11 @@ func resolveJDKWith(e jdkEnv, major int) (string, error) { } // Which Java a project needs is a MODEL setting, so a user staring at this // has a second way out that a bare "install a JDK" hides: build for a release - // they already have. Only say so when the requirement is not the default, - // because below 11.14 that is the only supported value and the advice would - // be noise. - if major != DefaultJavaMajor { + // they already have. Only say so when the requirement is above the default: + // below 11.14 the default is the only supported value, and a Mendix 9 + // project (JDK 17) has no JavaVersion setting at all, so the statement would + // change nothing (mendixlabs/mxcli#1437). + if major > DefaultJavaMajor { msg += fmt.Sprintf("\n This is the project's own Settings > Model > JavaVersion. To build for a"+ "\n release you already have instead: mxcli -p .mpr -c \"alter settings runtime ( JavaVersion: '%d' )\"", DefaultJavaMajor) } diff --git a/cmd/mxcli/docker/javaversion.go b/cmd/mxcli/docker/javaversion.go index bf8de43941..a4eb9b14a5 100644 --- a/cmd/mxcli/docker/javaversion.go +++ b/cmd/mxcli/docker/javaversion.go @@ -8,11 +8,26 @@ import ( "strings" ) -// DefaultJavaMajor is the Java release assumed when a project does not say which -// one it wants, or cannot be read. Every Mendix 9/10/11 project up to 11.13 uses -// it, so it is also the answer for the overwhelming majority of projects. +// DefaultJavaMajor is the Java release assumed when a project cannot be read, or +// when a Mendix 10/11 project does not say which one it wants. Every Mendix 10/11 +// project up to 11.13 uses it. Mendix 9 is the exception: see mendix9JavaMajor. const DefaultJavaMajor = 21 +// mendix9JavaMajor is the JDK for a Mendix 9 project. 9.x supports Java 11 and +// 17 only (9.24 LTS: 17) and its model has no JavaVersion setting, so the +// release cannot be read and 21 is wrong: mxbuild 9.24's Gradle 7.6 rejects it +// with "Unsupported class file major version 65" (mendixlabs/mxcli#1437). +const mendix9JavaMajor = 17 + +// defaultJavaMajorForMendix is the JDK assumed for a project of the given Mendix +// major version that stores no JavaVersion. Zero (unknown) keeps DefaultJavaMajor. +func defaultJavaMajorForMendix(mendixMajor int) int { + if mendixMajor == 9 { + return mendix9JavaMajor + } + return DefaultJavaMajor +} + // ProjectJavaMajor reads the Java release a project is built for. // // Mendix 11.14 is the first version whose blank app asks for something other @@ -26,8 +41,9 @@ const DefaultJavaMajor = 21 // JavaMajorVersion "21" from 11.12 — which settingsoverlay already normalises, // so both spellings arrive here as a bare number. // -// A project that cannot be read, or whose value is not a number, reports -// DefaultJavaMajor and false. The caller then behaves exactly as mxcli did +// A project that stores no numeric value reports the default for its Mendix +// version (defaultJavaMajorForMendix) and false; one that cannot be read +// reports DefaultJavaMajor and false. The caller then behaves exactly as mxcli did // before: guessing a different JDK from an unreadable model would trade a clear // failure for a confusing one. func ProjectJavaMajor(projectPath string) (int, bool) { @@ -43,7 +59,11 @@ func ProjectJavaMajor(projectPath string) (int, bool) { } n, ok := ParseJavaMajor(ps.Model.JavaVersion) if !ok { - return DefaultJavaMajor, false + mendixMajor := 0 + if pv := reader.ProjectVersion(); pv != nil { + mendixMajor = pv.MajorVersion + } + return defaultJavaMajorForMendix(mendixMajor), false } return n, true } diff --git a/cmd/mxcli/docker/javaversion_test.go b/cmd/mxcli/docker/javaversion_test.go index e6d55763c2..3953a879e2 100644 --- a/cmd/mxcli/docker/javaversion_test.go +++ b/cmd/mxcli/docker/javaversion_test.go @@ -9,6 +9,7 @@ package docker import ( + "errors" "strings" "testing" ) @@ -117,6 +118,63 @@ func TestJavaMajorOrDefault(t *testing.T) { } } +// mendixlabs/mxcli#1437: a Mendix 9 model stores no JavaVersion at all, and the +// fallback was a flat 21 — so `run --local` skipped a JDK 17 JAVA_HOME and +// mxbuild 9.24's Gradle failed with "Unsupported class file major version 65". +// The fallback must come from the project's Mendix version. +func TestProjectJavaMajorDefaultsToJava17ForMendix9(t *testing.T) { + p := v1Fixture(t) // Mendix 9.24.30, no JavaVersion setting + got, fromProject := ProjectJavaMajor(p) + if got != 17 || fromProject { + t.Errorf("ProjectJavaMajor(9.24 project) = (%d, %v), want (17, false)", got, fromProject) + } +} + +func TestDefaultJavaMajorForMendixVersion(t *testing.T) { + tests := []struct { + major int + want int + }{ + {9, 17}, + {10, DefaultJavaMajor}, + {11, DefaultJavaMajor}, + {0, DefaultJavaMajor}, // unknown version: behave as before + } + for _, tc := range tests { + if got := defaultJavaMajorForMendix(tc.major); got != tc.want { + t.Errorf("defaultJavaMajorForMendix(%d) = %d, want %d", tc.major, got, tc.want) + } + } +} + +// The "build for a release you already have" hint suggests lowering JavaVersion +// to 21. For a Mendix 9 project (needs 17) that advice is a no-op: 9.x has no +// JavaVersion setting, and 21 is not a JDK it can build with. +func TestResolveJDKDoesNotSuggestAlterSettingsBelowDefault(t *testing.T) { + env := jdkEnv{ + goos: "linux", + getenv: func(string) string { return "" }, + lookPath: func(string) (string, error) { return "", errors.New("not found") }, + evalSymlinks: func(p string) (string, error) { return p, nil }, + isJDK: func(string, int) bool { return false }, + glob: func(string) ([]string, error) { return nil, nil }, + searchPaths: func(int) []string { return nil }, + versionOf: func(string) string { return "" }, + } + _, err := resolveJDKWith(env, 17) + if err == nil { + t.Fatal("expected JDK 17 not found") + } + if strings.Contains(err.Error(), "alter settings") { + t.Errorf("JDK 17 error suggests alter settings, which no-ops on Mendix 9.x:\n%v", err) + } + // A project asking above the default can still be steered down to it. + _, err = resolveJDKWith(env, 25) + if err == nil || !strings.Contains(err.Error(), "alter settings") { + t.Errorf("JDK 25 error should keep the alter settings hint, got: %v", err) + } +} + func TestProjectJavaMajorFallsBackOnAnUnreadableProject(t *testing.T) { got, fromProject := ProjectJavaMajor("/nonexistent/project.mpr") if got != DefaultJavaMajor || fromProject { diff --git a/cmd/mxcli/main.go b/cmd/mxcli/main.go index 78479a7f1c..74d08c36e2 100644 --- a/cmd/mxcli/main.go +++ b/cmd/mxcli/main.go @@ -366,6 +366,7 @@ func init() { // Describe command flags describeCmd.Flags().StringP("format", "f", "mdl", "Output format: mdl, json, mermaid, elk, svg") + describeCmd.Flags().Bool("all", false, "With module: describe everything in the module (DESCRIBE MODULE ... WITH ALL), not only the module and its roles") // Search command flags searchCmd.Flags().StringP("format", "f", "table", "Output format: table, names (for JSON use --json; \"json\" is accepted as a deprecated alias)") diff --git a/docs-site/src/guides/marketplace.md b/docs-site/src/guides/marketplace.md index 97242be174..db36673d88 100644 --- a/docs-site/src/guides/marketplace.md +++ b/docs-site/src/guides/marketplace.md @@ -131,23 +131,25 @@ If you take that route the command states plainly that the project is now v1. ## Updating an existing module -Updating a module that is **already in the project is not done automatically**. `install` detects it, reports the installed and target versions, and stops: +Updating a module that is **already in the project is not done by `install`**. `install` detects it, reports the installed and target versions, names the command that does the update, and stops: ```text Module "DatabaseConnector" is already installed (version 7.0.1). -Target version: 7.0.3. -In-place module updates are not applied automatically (they can discard local -edits and change persistent-entity IDs, which loses data). Update via Studio Pro. +Target: version 7.0.3. +It is not updated by install (an update can discard local edits). To update it, +review what would change, then apply it with entity identity and role grants preserved: + mxcli marketplace diff 2888 -p app.mpr --to 7.0.3 + mxcli marketplace update 2888 -p app.mpr --to 7.0.3 ``` -Before you update in Studio Pro, the question worth answering is **whether anyone has edited the module since it was installed** — because the update will not ask. That is what `marketplace diff` is for; see below. +Before you update, the question worth answering is **whether anyone has edited the module since it was installed** — because the update will not ask. That is what `marketplace diff` is for; see below. Two reasons make automatic in-place module updates unsafe: 1. **Local edits.** Teams sometimes modify a marketplace module after importing it; a blind re-import would discard those changes. 2. **Persistent-entity IDs.** A fresh import assigns new entity `$ID`s. The runtime database keys data by entity ID, so re-importing a module with persistent entities would make the runtime treat them as *different* entities — **losing data**. -Studio Pro's Marketplace **Update** performs an ID-preserving merge that the `mx` CLI does not expose, so module updates are left to Studio Pro for now. +Studio Pro's Marketplace **Update** performs an ID-preserving merge that the `mx` CLI does not expose; `marketplace update` (below) is mxcli's equivalent, as a separate command so that `install` never updates implicitly. ## Updating a module (`marketplace update`) diff --git a/docs-site/src/reference/capabilities.md b/docs-site/src/reference/capabilities.md index 790bf5f53e..4f7ec1cb03 100644 --- a/docs-site/src/reference/capabilities.md +++ b/docs-site/src/reference/capabilities.md @@ -144,7 +144,7 @@ Everything mxcli can do, organized by use case. | REST query parameters | Requires Mendix 11.0+ | Build query string manually on 10.x | | Concurrent editing | Not supported — a file-based write is refused while Studio Pro has the project open (`.mpr.lock` present) | Close Studio Pro before mxcli writes, or write through it with `--mcp`; `exec --force` overrides | | Widget template drift | CE0463 on version mismatch | MPK augmentation handles most cases | -| Marketplace module update | Existing modules are reported, not updated in place | Update via Studio Pro (preserves local edits and entity IDs) | +| Marketplace module update | `install` reports an existing module and does not update it | `mxcli marketplace diff` / `update -p app.mpr --to ` (preserves entity identity and role grants; local edits are refused or saved with `--save-edits`) | | 47 of 52 metamodel domains | Not yet implemented | REST, OData write, etc. pending | ## Version Compatibility diff --git a/mdl-examples/bug-tests/1410-flows-on-mendix-9.mdl b/mdl-examples/bug-tests/1410-flows-on-mendix-9.mdl new file mode 100644 index 0000000000..6518ef44ac --- /dev/null +++ b/mdl-examples/bug-tests/1410-flows-on-mendix-9.mdl @@ -0,0 +1,53 @@ +mdl 1; +-- mendixlabs/mxcli#1410 — microflows and nanoflows could not be created or +-- altered in a Mendix 9 project (and 10.0–10.14 for any flow): +-- +-- this Mendix 9.24.2 project cannot store Microflows$Microflow.StableId +-- (Mendix 10.2.0+); Microflows$SequenceFlow.CaseValues (Mendix 10.15.0+) +-- +-- The flow writers now emit the shape the project's version stores: no +-- StableId below 10.2, the decision case as a single NewCaseValue below 10.15, +-- Origin/DestinationBezierVector instead of a BezierCurve Line below 10.16. +-- Measured: this script on a Studio Pro 9.24.30 app and on the same app after +-- `mx convert` to 10.12.19 checks at 0 errors; the old shape forced onto 9.24 +-- is 8 errors (CE0079 / CE0773 — the decision's cases are not read). +-- +-- Verify (on a copy of a Mendix 9.x or 10.0–10.14 project): +-- mxcli exec 1410-flows-on-mendix-9.mdl -p app.mpr +-- mxcli docker check -p app.mpr + +create module R1410; + +create entity R1410.Item ( + "Count": integer +); + +create microflow R1410.MF_Empty () +begin + return; +end; + +create microflow R1410.MF_Decide ($Item: R1410.Item) returns boolean +begin + declare $Result boolean = false; + if $Item/"Count" > 10 then + set $Result = true; + log info node 'R1410' 'big'; + else + log info node 'R1410' 'small'; + end if; + return $Result; +end; + +create nanoflow R1410.NF_Decide ($Item: R1410.Item) returns boolean +begin + if $Item/"Count" > 10 then + return true; + else + return false; + end if; +end; + +alter microflow R1410.MF_Decide { + insert after $Result begin log info node 'R1410' 'declared'; end; +}; diff --git a/mdl-examples/bug-tests/1413-set-on-aggregate-output.mdl b/mdl-examples/bug-tests/1413-set-on-aggregate-output.mdl new file mode 100644 index 0000000000..409511c3f2 --- /dev/null +++ b/mdl-examples/bug-tests/1413-set-on-aggregate-output.mdl @@ -0,0 +1,31 @@ +mdl 1; +-- ============================================================================ +-- mendixlabs/mxcli#1413: SET on a variable an aggregate produced +-- ============================================================================ +-- +-- `check -p` passed, `exec` refused the microflow with +-- +-- variable 'Sum' is not declared +-- +-- The validate pass declared an aggregate's output variable; the build pass +-- (addAggregateListAction) never did, so the SET that followed it failed only +-- at exec time. Both microflows below must exec cleanly. +-- ============================================================================ + +create module Bug1413; + +create persistent entity Bug1413.EntA ( Amount: Integer ); + +create microflow Bug1413.MF_Sum ($List: list of Bug1413.EntA) returns Integer +begin + $Sum = sum $List by Amount; + set $Sum = $Sum + 1; + return $Sum; +end; + +create microflow Bug1413.MF_Count ($List: list of Bug1413.EntA) returns Integer +begin + $N = count $List; + set $N = $N + 1; + return $N; +end; diff --git a/mdl-examples/bug-tests/1418-import-mapping-forward-ref.fail.mdl b/mdl-examples/bug-tests/1418-import-mapping-forward-ref.fail.mdl new file mode 100644 index 0000000000..ea1d4707da --- /dev/null +++ b/mdl-examples/bug-tests/1418-import-mapping-forward-ref.fail.mdl @@ -0,0 +1,27 @@ +-- Bug mendixlabs/mxcli#1418: exec writes an import-mapping microflow with a +-- null entity and the .mpr can no longer be loaded. +-- +-- NEGATIVE TEST (.fail.mdl) — EXPECTED to fail `mxcli check` (MDL-ORDER01). +-- +-- Symptom: `check --references` and `exec` both succeeded; the microflow was +-- stored with VariableType.Entity = "" because the mapping did not exist yet +-- when the flow was built, and `mx check` then died loading the project: +-- ArgumentNullException: Value cannot be null. (Parameter 'value') +-- at Mendix.Modeler.DataTypes.EntityType.set_EntityId +-- +-- Fix: MDL-ORDER01 reports the forward reference; exec refuses an +-- `import from mapping` whose mapping does not resolve; the writer refuses an +-- import activity without a result entity. +-- +-- Correct form: move `create import mapping` above the microflow. + +create json structure MyFirstModule.JSON_D sample '{"code": "NL", "name": "Netherlands"}'; +create persistent entity MyFirstModule.CountryD ( Code: string, Name: string ); + +create microflow MyFirstModule.MF_D ( $P: string ) +begin + $R = import from mapping MyFirstModule.IMM_D($P); +end; + +create import mapping MyFirstModule.IMM_D with json structure MyFirstModule.JSON_D +{ create MyFirstModule.CountryD { Code = code, Name = name } }; diff --git a/mdl-examples/bug-tests/1419-image-collection-same-script-ref.mdl b/mdl-examples/bug-tests/1419-image-collection-same-script-ref.mdl new file mode 100644 index 0000000000..55abfd73bd --- /dev/null +++ b/mdl-examples/bug-tests/1419-image-collection-same-script-ref.mdl @@ -0,0 +1,26 @@ +mdl 1; + +/** + * mendixlabs/mxcli#1419: exec's reference pre-check did not see an image + * collection created earlier in the same script. check reported + * image collection not found: ModA.Icons (referenced as ModA.Icons.pix) + * and exec refused: "Refusing to execute: 1 unresolved reference(s) above. + * Nothing was written." Split into two scripts, the same statements passed. + * + * The image file path is resolved from the current working directory, as in + * doctype-tests/19-image-collection-examples.mdl. + */ + +create module Bug1419; + +create image collection Bug1419.Icons { + image pix ( File: '../../docs/images/mxcli-cli.png' ) +}; + +create page Bug1419.IconPage ( + Title: 'Icon page', Layout: Atlas_Core.Atlas_Default +) { + placeholder Main { + image img1 (Image: 'Bug1419.Icons.pix') + } +}; diff --git a/mdl-examples/bug-tests/1423-odata-expose-quoted-member-name.mdl b/mdl-examples/bug-tests/1423-odata-expose-quoted-member-name.mdl new file mode 100644 index 0000000000..a8b209bfe4 --- /dev/null +++ b/mdl-examples/bug-tests/1423-odata-expose-quoted-member-name.mdl @@ -0,0 +1,29 @@ +mdl 1; + +/** + * mendixlabs/mxcli#1423: published OData `expose ("Description")` stored the + * double quotes in the attribute name. check and exec were clean; mx check said + * [CE1613] "The selected attribute 'ModA.EntA."Description"' no longer exists." + * and describe printed `"Description" as '"Description"'`. + * + * The quotes are identifier quoting, as in `create persistent entity`. After the + * fix describe prints `Description as 'Description'`. + */ + +create module Bug1423; + +create persistent entity Bug1423.EntA ( + Name: string(200) unique error message 'Name must be unique', + Description: string(200) +); + +create published odata service Bug1423.Pub ( + path: 'odata/bug1423/', + version: '1.0.0', + ODataVersion: OData4, + namespace: 'Bug1423.Ns' +) +{ + publish entity Bug1423.EntA as 'EntAs' (ReadMode: source) + expose (Name as 'Name' (KEY), "Description"); +}; diff --git a/mdl-examples/bug-tests/1427-user-role-check-security-false.mdl b/mdl-examples/bug-tests/1427-user-role-check-security-false.mdl new file mode 100644 index 0000000000..a1a4ab84df --- /dev/null +++ b/mdl-examples/bug-tests/1427-user-role-check-security-false.mdl @@ -0,0 +1,21 @@ +mdl 1; +-- mendixlabs/mxcli#1427: `describe user role` omitted `CheckSecurity` when it +-- was false. A new user role is written with CheckSecurity true, so describe -> +-- drop -> replay silently re-enabled security checking. +-- +-- Verify: +-- +-- mxcli exec 1427-user-role-check-security-false.mdl -p app.mpr +-- mxcli -p app.mpr -c "describe user role Bug1427_User" +-- -> output contains `CheckSecurity: false` +-- save that output, `drop user role Bug1427_User;`, exec the saved output +-- mxcli -p app.mpr -c "show user roles" +-- -> Bug1427_User still shows Check Security: No + +create module Bug1427; +create module role Bug1427.RoleA; + +create or modify user role Bug1427_User ( + ModuleRoles: (Bug1427.RoleA), + CheckSecurity: false +); diff --git a/mdl-examples/bug-tests/1430-list-sort-inherited-attribute.mdl b/mdl-examples/bug-tests/1430-list-sort-inherited-attribute.mdl new file mode 100644 index 0000000000..811a68b750 --- /dev/null +++ b/mdl-examples/bug-tests/1430-list-sort-inherited-attribute.mdl @@ -0,0 +1,27 @@ +mdl 1; +-- ============================================================================ +-- mendixlabs/mxcli#1430: list-operation sort on an inherited attribute +-- ============================================================================ +-- +-- `$L = sort $BookChunkList by Similarity desc;` where Similarity is declared on +-- the generalization was written as `Module.BookChunk.Similarity` and mxbuild +-- rejected it: +-- +-- CE1613 "The selected attribute '...BookChunk.Similarity' no longer exists." +-- +-- The sort must be qualified with the DECLARING entity (Bug1430.Chunk), as the +-- retrieve `sort by` already is. Expected after exec: describe shows the sort, +-- and `mx check` reports no CE1613. +-- ============================================================================ + +create module Bug1430; + +create persistent entity Bug1430.Chunk ( Similarity: Decimal ); + +create persistent entity Bug1430.BookChunk extends Bug1430.Chunk ( Title: String(100) ); + +create microflow Bug1430.MF_SortInherited ($BookChunkList: list of Bug1430.BookChunk) returns list of Bug1430.BookChunk +begin + $L = sort $BookChunkList by Similarity desc; + return $L; +end; diff --git a/mdl-examples/bug-tests/1445-system-java-action-lookup.mdl b/mdl-examples/bug-tests/1445-system-java-action-lookup.mdl new file mode 100644 index 0000000000..144bd9235c --- /dev/null +++ b/mdl-examples/bug-tests/1445-system-java-action-lookup.mdl @@ -0,0 +1,13 @@ +mdl 1; +-- mendixlabs/mxcli#1445: System Java actions are platform built-ins with no +-- stored unit. `list java actions` showed them, but describe and the microflow +-- builder's lookup said "java action not found", and exec warned +-- "could not look up Java action System.VerifyPassword ... (entity type params +-- will be empty)". Expected: exec creates the microflow with no lookup warning, +-- and `describe java action System.VerifyPassword` prints its signature. +create or modify microflow MyFirstModule.MF_Bug1445_VerifyPassword () +returns Boolean as $Ok +begin + $Ok = call java action System.VerifyPassword(userName = 'a', password = 'b'); + return $Ok; +end; diff --git a/mdl/backend/modelsdk/flow_version_keys_test.go b/mdl/backend/modelsdk/flow_version_keys_test.go new file mode 100644 index 0000000000..1203b2d2d8 --- /dev/null +++ b/mdl/backend/modelsdk/flow_version_keys_test.go @@ -0,0 +1,236 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "os" + "path/filepath" + "testing" + + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" + "go.mongodb.org/mongo-driver/v2/bson" +) + +// mendixlabs/mxcli#1410: on a Mendix 9 project every flow write was refused — +// +// this Mendix 9.24.2 project cannot store Microflows$Microflow.StableId +// (Mendix 10.2.0+); Microflows$SequenceFlow.CaseValues (Mendix 10.15.0+) +// +// The flow writers emitted the newest shape whatever the project's version: +// a StableId (introduced 10.2.0), the CaseValues list (introduced 10.15.0, +// replacing the single case stored as NewCaseValue) and a BezierCurve Line +// (introduced 10.16.0, replacing Origin/DestinationBezierVector). The property +// floor (canon.StripUndeclaredProperties) is right to refuse a key the version +// does not declare when it holds a value; the writer has to emit the shape the +// version has. The pre-10.15 shape is measured from a Studio Pro 9.24.30 +// document (Administration.ChangeMyPassword): +// +// {"$Type": "Microflows$SequenceFlow", "DestinationBezierVector": "-30;0", +// "DestinationConnectionIndex": 3, "DestinationPointer": …, "IsErrorHandler": false, +// "NewCaseValue": {"$Type": "Microflows$EnumerationCase", "Value": "true"}, +// "OriginBezierVector": "15;0", "OriginConnectionIndex": 1, "OriginPointer": …} +// +// and the version floors are the Model SDK's (mendixmodelsdk 4.116.0, +// StructureVersionInfo): Microflow.stableId 10.2.0, SequenceFlow.caseValue +// deleted / caseValues introduced 10.15.0, Flow.line 10.16.0. + +// copyV9Fixture copies the Mendix 9.24 v1 project the modelsdk/mpr tests use. +func copyV9Fixture(t *testing.T) string { + t.Helper() + dst := t.TempDir() + if err := os.CopyFS(dst, os.DirFS("../../../modelsdk/mpr/testdata/v1-project")); err != nil { + t.Fatalf("copy fixture: %v", err) + } + return filepath.Join(dst, "App.mpr") +} + +// decisionFlow builds start -> decision -(true)-> end, -(false)-> end, with a +// hand-drawn curve on the true branch. +func decisionFlow() *microflows.MicroflowObjectCollection { + obj := func(id string, x int) microflows.BaseMicroflowObject { + return microflows.BaseMicroflowObject{ + BaseElement: model.BaseElement{ID: model.ID(id)}, + Position: model.Point{X: x, Y: 100}, + Size: model.Size{Width: 30, Height: 30}, + } + } + start := µflows.StartEvent{BaseMicroflowObject: obj("a1410000-0000-0000-0000-000000000001", 0)} + split := µflows.ExclusiveSplit{ + BaseMicroflowObject: obj("a1410000-0000-0000-0000-000000000002", 100), + Caption: "big?", + SplitCondition: µflows.ExpressionSplitCondition{Expression: "1 > 0"}, + } + endT := µflows.EndEvent{BaseMicroflowObject: obj("a1410000-0000-0000-0000-000000000003", 200)} + endF := µflows.EndEvent{BaseMicroflowObject: obj("a1410000-0000-0000-0000-000000000004", 300)} + flow := func(id, from, to string, cv microflows.CaseValue) *microflows.SequenceFlow { + return µflows.SequenceFlow{ + BaseElement: model.BaseElement{ID: model.ID(id)}, + OriginID: model.ID(from), DestinationID: model.ID(to), + OriginConnectionIndex: 1, DestinationConnectionIndex: 3, + CaseValue: cv, + } + } + curved := flow("a1410000-0000-0000-0000-000000000012", string(split.ID), string(endT.ID), µflows.EnumerationCase{Value: "true"}) + curved.OriginControlVector, curved.DestinationControlVector = "15;0", "-30;0" + return µflows.MicroflowObjectCollection{ + Objects: []microflows.MicroflowObject{start, split, endT, endF}, + Flows: []*microflows.SequenceFlow{ + flow("a1410000-0000-0000-0000-000000000011", string(start.ID), string(split.ID), nil), + curved, + flow("a1410000-0000-0000-0000-000000000013", string(split.ID), string(endF.ID), µflows.EnumerationCase{Value: "false"}), + }, + } +} + +// assertPre1015Flows checks every sequence flow of a stored flow document has +// the Mendix 9 shape: a single NewCaseValue, bezier vectors, no Line. +func assertPre1015Flows(t *testing.T, raw bson.Raw, wantCases map[string]string) { + t.Helper() + flows, ok := raw.Lookup("Flows").ArrayOK() + if !ok { + t.Fatalf("no Flows list") + } + vals, _ := flows.Values() + n := 0 + for _, v := range vals { + d, ok := v.DocumentOK() + if !ok { + continue + } + if typ, _ := d.Lookup("$Type").StringValueOK(); typ != "Microflows$SequenceFlow" { + continue + } + n++ + for _, key := range []string{"CaseValues", "Line", "LineType"} { + if _, err := d.LookupErr(key); err == nil { + t.Errorf("sequence flow carries %s, which Mendix 9 does not declare", key) + } + } + cv, ok := d.Lookup("NewCaseValue").DocumentOK() + if !ok { + t.Errorf("sequence flow has no NewCaseValue document — the case Mendix 9 stores") + continue + } + for _, key := range []string{"OriginBezierVector", "DestinationBezierVector"} { + if _, ok := d.Lookup(key).StringValueOK(); !ok { + t.Errorf("sequence flow has no %s", key) + } + } + id := blobID(d.Lookup("$ID")) + typ, _ := cv.Lookup("$Type").StringValueOK() + val, _ := cv.Lookup("Value").StringValueOK() + if want, ok := wantCases[id]; ok { + if typ != "Microflows$EnumerationCase" || val != want { + t.Errorf("flow %s case = %s %q, want EnumerationCase %q", id, typ, val, want) + } + if want == "true" { + ov, _ := d.Lookup("OriginBezierVector").StringValueOK() + dv, _ := d.Lookup("DestinationBezierVector").StringValueOK() + if ov != "15;0" || dv != "-30;0" { + t.Errorf("curved flow vectors = %q/%q, want 15;0/-30;0", ov, dv) + } + } + } else if typ != "Microflows$NoCase" { + t.Errorf("flow %s case = %s, want NoCase", id, typ) + } + } + if n != 3 { + t.Errorf("stored %d sequence flows, want 3", n) + } +} + +func blobID(v bson.RawValue) string { + _, data, ok := v.BinaryOK() + if !ok { + return "" + } + return types.BlobToUUID(data) +} + +func TestFlowWritesOnMendix9_1410(t *testing.T) { + b := New() + if err := b.Connect(copyV9Fixture(t)); err != nil { + t.Fatalf("connect: %v", err) + } + t.Cleanup(func() { _ = b.Disconnect() }) + if pv := b.ProjectVersion(); pv == nil || pv.MajorVersion != 9 { + t.Fatalf("fixture version = %+v, want a Mendix 9 project", pv) + } + mod, err := b.GetModuleByName("NanoflowCommons") + if err != nil || mod == nil { + t.Fatalf("GetModuleByName: %v", err) + } + cases := map[string]string{ + "a1410000-0000-0000-0000-000000000012": "true", + "a1410000-0000-0000-0000-000000000013": "false", + } + + t.Run("create microflow", func(t *testing.T) { + mf := µflows.Microflow{Name: "MF_Decide1410", ObjectCollection: decisionFlow()} + mf.ContainerID = mod.ID + if err := b.CreateMicroflow(mf); err != nil { + t.Fatalf("CreateMicroflow on Mendix 9: %v", err) + } + raw, err := b.GetRawUnitBytes(mf.ID) + if err != nil { + t.Fatalf("GetRawUnitBytes: %v", err) + } + if _, err := bson.Raw(raw).LookupErr("StableId"); err == nil { + t.Errorf("microflow carries StableId, introduced in Mendix 10.2.0") + } + assertPre1015Flows(t, raw, cases) + + // It reads back as written: the case and the curve. + got, err := b.GetMicroflow(mf.ID) + if err != nil { + t.Fatalf("GetMicroflow: %v", err) + } + for _, f := range got.ObjectCollection.Flows { + want, ok := cases[string(f.ID)] + if !ok { + continue + } + ec, isEnum := f.CaseValue.(*microflows.EnumerationCase) + if !isEnum || ec.Value != want { + t.Errorf("flow %s reads back with case %#v, want %q", f.ID, f.CaseValue, want) + } + if want == "true" && (f.OriginControlVector != "15;0" || f.DestinationControlVector != "-30;0") { + t.Errorf("curved flow reads back as %q/%q, want 15;0/-30;0", f.OriginControlVector, f.DestinationControlVector) + } + } + }) + + t.Run("create nanoflow", func(t *testing.T) { + nf := µflows.Nanoflow{Name: "NF_Decide1410", ObjectCollection: decisionFlow()} + nf.ContainerID = mod.ID + if err := b.CreateNanoflow(nf); err != nil { + t.Fatalf("CreateNanoflow on Mendix 9: %v", err) + } + raw, err := b.GetRawUnitBytes(nf.ID) + if err != nil { + t.Fatalf("GetRawUnitBytes: %v", err) + } + assertPre1015Flows(t, raw, cases) + }) + + t.Run("alter splices a Mendix 9 flow", func(t *testing.T) { + // The splice encodes each new flow with this, and setCase moves the + // case of a stored flow under the key this writes. + d, err := codecMicroflowDeps{b: b}.SerializeSequenceFlow(µflows.SequenceFlow{ + BaseElement: model.BaseElement{ID: model.ID("a1410000-0000-0000-0000-000000000021")}, + CaseValue: µflows.EnumerationCase{Value: "true"}, + }) + if err != nil { + t.Fatalf("SerializeSequenceFlow: %v", err) + } + keys := map[string]bool{} + for _, e := range d { + keys[e.Key] = true + } + if keys["CaseValues"] || keys["Line"] || !keys["NewCaseValue"] || !keys["OriginBezierVector"] { + t.Errorf("spliced flow keys = %v, want NewCaseValue and the bezier vectors, no CaseValues/Line", keys) + } + }) +} diff --git a/mdl/backend/modelsdk/flow_version_shape_test.go b/mdl/backend/modelsdk/flow_version_shape_test.go new file mode 100644 index 0000000000..23ddde7346 --- /dev/null +++ b/mdl/backend/modelsdk/flow_version_shape_test.go @@ -0,0 +1,129 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "fmt" + "testing" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" + "go.mongodb.org/mongo-driver/v2/bson" +) + +// flowDocKeys returns the keys of one encoded document. +func flowDocKeys(t *testing.T, raw []byte) map[string]bool { + t.Helper() + elems, err := bson.Raw(raw).Elements() + if err != nil { + t.Fatalf("elements: %v", err) + } + out := map[string]bool{} + for _, e := range elems { + out[e.Key()] = true + } + return out +} + +// TestFlowShapeByVersion pins every version-gated key of the flow writers to +// the Model SDK's floor (mendixlabs/mxcli#1410), across the bands: 9.x, +// 10.0–10.1, 10.2–10.7, 10.8–10.11, 10.12–10.14, 10.15, 10.16, 10.17+. The +// 10.12 row is measured, besides 9.24: Administration.ChangeMyPassword after +// `mx convert` to 10.12.19 stores StableId and ReturnVariableName, a +// NewCaseValue, both bezier vectors and LineType "BezierCurve", and no +// parameter IsRequired/DefaultValue. +func TestFlowShapeByVersion(t *testing.T) { + type want struct { + stableID, returnVar bool + caseList, line, lineType, param bool + } + all := want{stableID: true, returnVar: true, caseList: true, line: true, param: true} + tests := []struct { + v flowVersion + want want + }{ + {flowVersion{9, 24}, want{}}, + {flowVersion{10, 1}, want{}}, + {flowVersion{10, 2}, want{stableID: true}}, + {flowVersion{10, 8}, want{stableID: true, lineType: true}}, + {flowVersion{10, 12}, want{stableID: true, returnVar: true, lineType: true}}, + {flowVersion{10, 14}, want{stableID: true, returnVar: true, lineType: true}}, + {flowVersion{10, 15}, want{stableID: true, returnVar: true, caseList: true, lineType: true}}, + {flowVersion{10, 16}, want{stableID: true, returnVar: true, caseList: true, line: true}}, + {flowVersion{10, 17}, all}, + {flowVersion{11, 6}, all}, + {newestFlowVersion, all}, + } + for _, tt := range tests { + t.Run(fmt.Sprintf("%d.%d", tt.v.major, tt.v.minor), func(t *testing.T) { + mf := µflows.Microflow{ + Name: "MF", + ObjectCollection: decisionFlow(), + Parameters: []*microflows.MicroflowParameter{{Name: "P", Type: µflows.BooleanType{}}}, + } + mf.ObjectCollection.AnnotationFlows = []*microflows.AnnotationFlow{{ + BaseElement: model.BaseElement{ID: "a1410000-0000-0000-0000-000000000031"}, + OriginID: "a1410000-0000-0000-0000-000000000001", + DestinationID: "a1410000-0000-0000-0000-000000000002", + }} + gm := microflowToGen(mf, tt.v) + gm.SetID("a1410000-0000-0000-0000-000000000000") + assignMicroflowIDs(gm) + raw, err := microflowEncoder(tt.v).Encode(gm) + if err != nil { + t.Fatalf("encode: %v", err) + } + check := func(what string, got, want bool) { + t.Helper() + if got != want { + t.Errorf("%s present = %v, want %v", what, got, want) + } + } + top := flowDocKeys(t, raw) + check("Microflow.StableId", top["StableId"], tt.want.stableID) + check("Microflow.ReturnVariableName", top["ReturnVariableName"], tt.want.returnVar) + + flows, _ := bson.Raw(raw).Lookup("Flows").Array().Values() + var seq, ann map[string]bool + for _, f := range flows { + d, ok := f.DocumentOK() + if !ok { + continue + } + switch typ, _ := d.Lookup("$Type").StringValueOK(); typ { + case "Microflows$SequenceFlow": + seq = flowDocKeys(t, d) + case "Microflows$AnnotationFlow": + ann = flowDocKeys(t, d) + } + } + for name, keys := range map[string]map[string]bool{"SequenceFlow": seq, "AnnotationFlow": ann} { + check(name+".Line", keys["Line"], tt.want.line) + check(name+".OriginBezierVector", keys["OriginBezierVector"], !tt.want.line) + check(name+".DestinationBezierVector", keys["DestinationBezierVector"], !tt.want.line) + check(name+".LineType", keys["LineType"], tt.want.lineType) + } + check("SequenceFlow.CaseValues", seq["CaseValues"], tt.want.caseList) + check("SequenceFlow.NewCaseValue", seq["NewCaseValue"], !tt.want.caseList) + check("SequenceFlow.CaseValue (the SDK name, never stored)", seq["CaseValue"], false) + + objs, _ := bson.Raw(raw).Lookup("ObjectCollection", "Objects").Array().Values() + found := false + for _, o := range objs { + d, ok := o.DocumentOK() + if !ok { + continue + } + if typ, _ := d.Lookup("$Type").StringValueOK(); typ == "Microflows$MicroflowParameter" { + found = true + keys := flowDocKeys(t, d) + check("MicroflowParameter.IsRequired", keys["IsRequired"], tt.want.param) + check("MicroflowParameter.DefaultValue", keys["DefaultValue"], tt.want.param) + } + } + if !found { + t.Errorf("no Microflows$MicroflowParameter written") + } + }) + } +} diff --git a/mdl/backend/modelsdk/java_read.go b/mdl/backend/modelsdk/java_read.go index 8ef40a3908..21fd0a8b94 100644 --- a/mdl/backend/modelsdk/java_read.go +++ b/mdl/backend/modelsdk/java_read.go @@ -29,6 +29,9 @@ func (b *Backend) ReadJavaActionByName(qualifiedName string) (*javaactions.JavaA moduleName, actionName := qualifiedName[:dot], qualifiedName[dot+1:] mod, err := b.GetModuleByName(moduleName) if err != nil || mod == nil { + if ja := systemJavaActionByName(moduleName, actionName); ja != nil { + return ja, nil + } return nil, fmt.Errorf("java action not found: %s", qualifiedName) } containers := b.containerSetForModule(string(mod.ID)) // module + nested folders @@ -41,9 +44,29 @@ func (b *Backend) ReadJavaActionByName(qualifiedName string) (*javaactions.JavaA return javaActionFromGen(u.Element, u.ContainerID), nil } } + if ja := systemJavaActionByName(moduleName, actionName); ja != nil { + return ja, nil + } return nil, fmt.Errorf("java action not found: %s", qualifiedName) } +// systemJavaActionByName resolves a System-module platform built-in (e.g. +// System.VerifyPassword), which has no stored unit. The list paths already +// synthesize these, so the by-name read must too or a listed action cannot be +// described or called (mendixlabs/mxcli#1445). Stored units win; this is only +// the fallback. +func systemJavaActionByName(moduleName, actionName string) *javaactions.JavaAction { + if moduleName != "System" { + return nil + } + for _, ja := range meta.BuildSystemJavaActionsFull() { + if ja.Name == actionName { + return ja + } + } + return nil +} + // ListJavaActionsFull reads every Java action unit into the fully-parsed semantic // form (parameters, types, return type) — what the catalog's java-actions builder // and SHOW JAVA ACTIONS need. (The virtual System-module java actions that the diff --git a/mdl/backend/modelsdk/java_system_test.go b/mdl/backend/modelsdk/java_system_test.go index b3855c7c9a..695c648c94 100644 --- a/mdl/backend/modelsdk/java_system_test.go +++ b/mdl/backend/modelsdk/java_system_test.go @@ -100,3 +100,30 @@ func TestListJavaActions_StillReturnsStoredActions(t *testing.T) { "the stored ones were dropped", len(got)) } } + +// mendixlabs/mxcli#1445: `list java actions` showed System.VerifyPassword, but +// `describe java action System.VerifyPassword` and the microflow builder's lookup +// both said "java action not found" — the by-name read only scanned stored units. +func TestReadJavaActionByName_ResolvesSystemBuiltIns(t *testing.T) { + b := New() + if err := b.Connect(copyFixture(t)); err != nil { + t.Fatalf("connect: %v", err) + } + t.Cleanup(func() { _ = b.Disconnect() }) + + for _, def := range meta.SystemJavaActions { + ja, err := b.ReadJavaActionByName("System." + def.Name) + if err != nil { + t.Errorf("ReadJavaActionByName(System.%s): %v", def.Name, err) + continue + } + if ja.Name != def.Name || len(ja.Parameters) != len(def.Parameters) { + t.Errorf("System.%s: got name %q with %d parameters, want %d", + def.Name, ja.Name, len(ja.Parameters), len(def.Parameters)) + } + } + // Control: an unknown System name must still be "not found". + if _, err := b.ReadJavaActionByName("System.NoSuchAction"); err == nil { + t.Error("System.NoSuchAction resolved; want not found") + } +} diff --git a/mdl/backend/modelsdk/microflow.go b/mdl/backend/modelsdk/microflow.go index 10f71a71b9..f6697429ce 100644 --- a/mdl/backend/modelsdk/microflow.go +++ b/mdl/backend/modelsdk/microflow.go @@ -334,6 +334,11 @@ func flowsFromGen(items []element.Element) []*microflows.SequenceFlow { if line, ok := g.Line().(*genMf.BezierCurve); ok && line != nil { f.OriginControlVector = line.OriginControlVector() f.DestinationControlVector = line.DestinationControlVector() + } else { + // Before 10.16 the same two vectors are the flow's own + // Origin/DestinationBezierVector (mendixlabs/mxcli#1410). + f.OriginControlVector = g.OriginBezierVector() + f.DestinationControlVector = g.DestinationBezierVector() } f.ID = model.ID(g.ID()) flows = append(flows, f) diff --git a/mdl/backend/modelsdk/microflow_annotation_test.go b/mdl/backend/modelsdk/microflow_annotation_test.go index 0dd81dc9f2..297041f587 100644 --- a/mdl/backend/modelsdk/microflow_annotation_test.go +++ b/mdl/backend/modelsdk/microflow_annotation_test.go @@ -48,7 +48,7 @@ func TestAnnotationFlowToGen(t *testing.T) { OriginID: model.ID("ann-1"), DestinationID: model.ID("act-1"), } - raw, err := (&codec.Encoder{}).Encode(annotationFlowToGen(af, 11)) + raw, err := (&codec.Encoder{}).Encode(annotationFlowToGen(af, newestFlowVersion)) if err != nil { t.Fatalf("encode: %v", err) } diff --git a/mdl/backend/modelsdk/microflow_idempotence_test.go b/mdl/backend/modelsdk/microflow_idempotence_test.go index 15aecb778d..603a9b5931 100644 --- a/mdl/backend/modelsdk/microflow_idempotence_test.go +++ b/mdl/backend/modelsdk/microflow_idempotence_test.go @@ -141,7 +141,7 @@ func rebuildMicroflow(t *testing.T, proj string, mf *microflows.Microflow) []byt } defer b.Disconnect() - gm := microflowToGen(mf, b.majorVersion()) + gm := microflowToGen(mf, b.flowVersion()) gm.SetID(element.ID(mf.ID)) assignMicroflowIDs(gm) contents, err := (&codec.Encoder{}).Encode(gm) diff --git a/mdl/backend/modelsdk/microflow_import_entity_guard.go b/mdl/backend/modelsdk/microflow_import_entity_guard.go new file mode 100644 index 0000000000..f28fb38531 --- /dev/null +++ b/mdl/backend/modelsdk/microflow_import_entity_guard.go @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "fmt" + + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// importMappingEntityError refuses an import-from-mapping activity without a +// result entity. importXmlActionToGen stores the entity as the activity's +// VariableType.Entity, and an empty one cannot be loaded: Mendix rebuilds it +// into an EntityIdentifier as it reads the unit and `mx check` dies with +// ArgumentNullException in EntityType.set_EntityId before validating anything +// (mendixlabs/mxcli#1418). The builder refuses a mapping it cannot resolve; +// this is the last line, for every path that reaches the encoder. Loop bodies +// are walked, as their activities are written the same way. +func importMappingEntityError(obj microflows.MicroflowObject) error { + switch o := obj.(type) { + case *microflows.ActionActivity: + ix, ok := o.Action.(*microflows.ImportXmlAction) + if !ok { + return nil + } + if ix.ResultHandling == nil || ix.ResultHandling.ResultEntityID == "" { + mapping := "(none)" + if ix.ResultHandling != nil && ix.ResultHandling.MappingID != "" { + mapping = string(ix.ResultHandling.MappingID) + } + return fmt.Errorf("refusing to write import from mapping %s: the activity has no result entity, "+ + "and Mendix cannot load a project holding one (EntityType.set_EntityId: value cannot be null). "+ + "The import mapping must exist, with a root entity, before the flow that calls it is written", mapping) + } + case *microflows.LoopedActivity: + if o.ObjectCollection != nil { + for _, inner := range o.ObjectCollection.Objects { + if err := importMappingEntityError(inner); err != nil { + return err + } + } + } + } + return nil +} + +// flowImportMappingEntityError applies importMappingEntityError to a flow's objects. +func flowImportMappingEntityError(oc *microflows.MicroflowObjectCollection) error { + if oc == nil { + return nil + } + for _, obj := range oc.Objects { + if err := importMappingEntityError(obj); err != nil { + return err + } + } + return nil +} diff --git a/mdl/backend/modelsdk/microflow_import_entity_guard_test.go b/mdl/backend/modelsdk/microflow_import_entity_guard_test.go new file mode 100644 index 0000000000..5a67acc25d --- /dev/null +++ b/mdl/backend/modelsdk/microflow_import_entity_guard_test.go @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// mendixlabs/mxcli#1418: an import-from-mapping activity stored with an empty +// VariableType.Entity makes the whole project unloadable: +// +// System.InvalidOperationException: An error occurred when trying to set the +// 'Entity' property of a Object type in a Microflow with ID ... +// ---> System.ArgumentNullException: Value cannot be null. (Parameter 'value') +// at Mendix.Modeler.DataTypes.EntityType.set_EntityId(EntityIdentifier value) +// +// (mx check, 11.6.6). Whatever the caller hands it, the writer refuses rather +// than store one — including inside a loop body, and on the splice path. +func importActivity(entity model.ID) *microflows.ActionActivity { + return µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: microflows.BaseMicroflowObject{ + BaseElement: model.BaseElement{ID: "act-1"}, + }}, + Action: µflows.ImportXmlAction{ + BaseElement: model.BaseElement{ID: "imp-1"}, + XmlDocumentVariable: "P", + ResultHandling: µflows.ResultHandlingMapping{ + BaseElement: model.BaseElement{ID: "rh-1"}, + MappingID: "MyFirstModule.IMM_D", + ResultEntityID: entity, + ResultVariable: "R", + SingleObject: true, + }, + }, + } +} + +func TestWriterRefusesImportMappingWithoutResultEntity(t *testing.T) { + inLoop := µflows.LoopedActivity{ + BaseMicroflowObject: microflows.BaseMicroflowObject{BaseElement: model.BaseElement{ID: "loop-1"}}, + ObjectCollection: µflows.MicroflowObjectCollection{ + Objects: []microflows.MicroflowObject{importActivity("")}, + }, + } + for name, obj := range map[string]microflows.MicroflowObject{ + "top level": importActivity(""), + "in a loop": inLoop, + } { + t.Run(name, func(t *testing.T) { + mf := µflows.Microflow{ + BaseElement: model.BaseElement{ID: "mf-1"}, + Name: "MF_D", + ObjectCollection: µflows.MicroflowObjectCollection{Objects: []microflows.MicroflowObject{obj}}, + } + b := &Backend{} + for op, err := range map[string]error{ + "CreateMicroflow": b.CreateMicroflow(mf), + "UpdateMicroflow": b.UpdateMicroflow(mf), + } { + if err == nil || !strings.Contains(err.Error(), "MyFirstModule.IMM_D") || !strings.Contains(err.Error(), "result entity") { + t.Errorf("%s: err = %v, want a refusal naming the mapping's missing result entity", op, err) + } + } + if _, err := (codecMicroflowDeps{b: b}).SerializeObject(obj); err == nil { + t.Errorf("SerializeObject: no error for an import activity without a result entity") + } + }) + } + + // Control: with the entity the guard stays out of the way. + if err := importMappingEntityError(importActivity("MyFirstModule.CountryD")); err != nil { + t.Fatalf("activity with a result entity refused: %v", err) + } +} diff --git a/mdl/backend/modelsdk/microflow_mutator_write.go b/mdl/backend/modelsdk/microflow_mutator_write.go index 43ff761eeb..58c0e68a29 100644 --- a/mdl/backend/modelsdk/microflow_mutator_write.go +++ b/mdl/backend/modelsdk/microflow_mutator_write.go @@ -69,6 +69,9 @@ func (d codecMicroflowDeps) DeclaresProperty(docType, key string) bool { } func (d codecMicroflowDeps) SerializeObject(obj microflows.MicroflowObject) (bson.D, error) { + if err := importMappingEntityError(obj); err != nil { + return nil, err + } el := microflowObjectToGen(obj) if el == nil { return nil, nil @@ -78,19 +81,16 @@ func (d codecMicroflowDeps) SerializeObject(obj microflows.MicroflowObject) (bso } func (d codecMicroflowDeps) SerializeSequenceFlow(f *microflows.SequenceFlow) (bson.D, error) { - el := sequenceFlowToGen(f, d.b.majorVersion()) + el := sequenceFlowToGen(f, d.b.flowVersion()) assignID(el) if sf, ok := el.(*genMf.SequenceFlow); ok { - for _, cv := range sf.CaseValuesItems() { - assignID(cv) - } - assignID(sf.Line()) + assignSequenceFlowIDs(sf) } return encodeFlowElementToD(el) } func (d codecMicroflowDeps) SerializeAnnotationFlow(f *microflows.AnnotationFlow) (bson.D, error) { - el := annotationFlowToGen(f, d.b.majorVersion()) + el := annotationFlowToGen(f, d.b.flowVersion()) assignID(el) if af, ok := el.(*genMf.AnnotationFlow); ok { assignID(af.Line()) @@ -105,10 +105,11 @@ func (d codecMicroflowDeps) SerializeDocument(declared any) (bson.D, error) { var el element.Element switch doc := declared.(type) { case *microflows.Microflow: - gm := microflowToGen(doc, d.b.majorVersion()) + fv := d.b.flowVersion() + gm := microflowToGen(doc, fv) gm.SetID(element.ID(doc.ID)) assignMicroflowIDs(gm) - contents, err := (&codec.Encoder{}).Encode(gm) + contents, err := microflowEncoder(fv).Encode(gm) if err != nil { return nil, err } @@ -121,7 +122,7 @@ func (d codecMicroflowDeps) SerializeDocument(declared any) (bson.D, error) { } return out, nil case *microflows.Nanoflow: - g := nanoflowToGen(doc, d.b.majorVersion()) + g := nanoflowToGen(doc, d.b.flowVersion()) g.SetID(element.ID(doc.ID)) assignNanoflowIDs(g) d.b.carryStoredNanoflowHeader(doc, g) diff --git a/mdl/backend/modelsdk/microflow_parameter_position_test.go b/mdl/backend/modelsdk/microflow_parameter_position_test.go index e38901c87c..d46ab57819 100644 --- a/mdl/backend/modelsdk/microflow_parameter_position_test.go +++ b/mdl/backend/modelsdk/microflow_parameter_position_test.go @@ -18,18 +18,18 @@ func TestMicroflowParameterToGenKeepsAuthoredPosition(t *testing.T) { Name: "Feedback", Position: &model.Point{X: -77, Y: 0}, } - g := microflowParameterToGen(authored, 0, 11).(*genMf.MicroflowParameter) + g := microflowParameterToGen(authored, 0, newestFlowVersion).(*genMf.MicroflowParameter) if got := g.RelativeMiddlePoint(); got != "-77;0" { t.Errorf("authored position = %q, want -77;0", got) } // Control: unannotated parameters still derive from the index. derived := µflows.MicroflowParameter{Name: "Feedback"} - g = microflowParameterToGen(derived, 0, 11).(*genMf.MicroflowParameter) + g = microflowParameterToGen(derived, 0, newestFlowVersion).(*genMf.MicroflowParameter) if got := g.RelativeMiddlePoint(); got != "200;53" { t.Errorf("derived position at index 0 = %q, want 200;53", got) } - g = microflowParameterToGen(derived, 1, 11).(*genMf.MicroflowParameter) + g = microflowParameterToGen(derived, 1, newestFlowVersion).(*genMf.MicroflowParameter) if got := g.RelativeMiddlePoint(); got != "300;53" { t.Errorf("derived position at index 1 = %q, want 300;53", got) } diff --git a/mdl/backend/modelsdk/microflow_readback.go b/mdl/backend/modelsdk/microflow_readback.go index 10d2bdba79..c7e91cef55 100644 --- a/mdl/backend/modelsdk/microflow_readback.go +++ b/mdl/backend/modelsdk/microflow_readback.go @@ -35,10 +35,11 @@ func (b *Backend) ReadBackMicroflow(mf *microflows.Microflow) (*microflows.Micro return nil, fmt.Errorf("ReadBackMicroflow: nil microflow") } encode := func(m *microflows.Microflow) ([]byte, error) { - gm := microflowToGen(m, b.majorVersion()) + fv := b.flowVersion() + gm := microflowToGen(m, fv) gm.SetID(element.ID(readBackID(m.ID))) assignMicroflowIDs(gm) - raw, err := (&codec.Encoder{}).Encode(gm) + raw, err := microflowEncoder(fv).Encode(gm) return b.completeAsStored(raw), err } written, err := encode(mf) @@ -70,7 +71,7 @@ func (b *Backend) ReadBackNanoflow(nf *microflows.Nanoflow) (*microflows.Nanoflo return nil, fmt.Errorf("ReadBackNanoflow: nil nanoflow") } encode := func(n *microflows.Nanoflow) ([]byte, error) { - g := nanoflowToGen(n, b.majorVersion()) + g := nanoflowToGen(n, b.flowVersion()) g.SetID(element.ID(readBackID(n.ID))) assignNanoflowIDs(g) raw, err := (&codec.Encoder{}).Encode(g) diff --git a/mdl/backend/modelsdk/microflow_roundtrip_flags_test.go b/mdl/backend/modelsdk/microflow_roundtrip_flags_test.go index cfd8e7a4b1..259100bd5b 100644 --- a/mdl/backend/modelsdk/microflow_roundtrip_flags_test.go +++ b/mdl/backend/modelsdk/microflow_roundtrip_flags_test.go @@ -18,7 +18,7 @@ import ( // the way out but never read back are silently reset on the return trip. func roundTripMicroflow(t *testing.T, mf *microflows.Microflow) *microflows.Microflow { t.Helper() - gm := microflowToGen(mf, 11) + gm := microflowToGen(mf, newestFlowVersion) raw, err := (&codec.Encoder{}).Encode(gm) if err != nil { t.Fatalf("encode: %v", err) @@ -229,7 +229,7 @@ func TestMicroflowRoundTrip_NoConcurrencyMessageIsInert(t *testing.T) { encMessage := func(t *testing.T, mf *microflows.Microflow) []byte { t.Helper() mf.ID = model.ID("mf-9") - raw, err := (&codec.Encoder{}).Encode(microflowToGen(mf, 11).ConcurrencyErrorMessage()) + raw, err := (&codec.Encoder{}).Encode(microflowToGen(mf, newestFlowVersion).ConcurrencyErrorMessage()) if err != nil { t.Fatalf("encode: %v", err) } diff --git a/mdl/backend/modelsdk/microflow_rule_split_write_test.go b/mdl/backend/modelsdk/microflow_rule_split_write_test.go index 0e12e17d71..116424cb8e 100644 --- a/mdl/backend/modelsdk/microflow_rule_split_write_test.go +++ b/mdl/backend/modelsdk/microflow_rule_split_write_test.go @@ -89,7 +89,7 @@ func TestMicroflowRoundTrip_RuleSplitCondition(t *testing.T) { // agree. Written as "Rule" the reference is invisible to Mendix and the decision is // CE0080 again, for a different reason. Assert the storage key on the raw document. func TestRuleSplitConditionUsesMicroflowStorageKey(t *testing.T) { - raw, err := (&codec.Encoder{}).Encode(microflowToGen(ruleSplitMicroflow(), 11)) + raw, err := (&codec.Encoder{}).Encode(microflowToGen(ruleSplitMicroflow(), newestFlowVersion)) if err != nil { t.Fatalf("encode: %v", err) } diff --git a/mdl/backend/modelsdk/microflow_write.go b/mdl/backend/modelsdk/microflow_write.go index 11e12fc60f..a26fc28722 100644 --- a/mdl/backend/modelsdk/microflow_write.go +++ b/mdl/backend/modelsdk/microflow_write.go @@ -143,21 +143,81 @@ func (b *Backend) majorVersion() int { return 11 } +// flowVersion is the Mendix version a flow document — microflow, nanoflow or +// rule — is written for. Flow properties changed shape within Mendix 10, so +// the major alone cannot gate them (mendixlabs/mxcli#1410: every flow write on +// a Mendix 9 project was refused for a StableId and a CaseValues list it +// cannot store, and 10.0–10.14 for the CaseValues list). +// +// The floors are the Model SDK's StructureVersionInfo (mendixmodelsdk +// 4.116.0), and the shapes are measured: a Studio Pro 9.24.30 document, and +// the same document after `mx convert` to 10.12.19. +type flowVersion struct{ major, minor int } + +// newestFlowVersion is what an unreadable project version is written as — +// the newest shape, as majorVersion() has always defaulted to 11. +var newestFlowVersion = flowVersion{major: 11} + +func (v flowVersion) atLeast(major, minor int) bool { + return v.major > major || v.major == major && v.minor >= minor +} + +// Microflow.stableId, introduced in 10.2.0. +func (v flowVersion) hasStableID() bool { return v.atLeast(10, 2) } + +// SequenceFlow.caseValues, introduced in 10.15.0 when the single caseValue +// (stored as NewCaseValue) was deleted. +func (v flowVersion) hasCaseValueList() bool { return v.atLeast(10, 15) } + +// Flow.line, introduced in 10.16.0 when Origin/DestinationBezierVector were +// deleted. Flow.lineType lived between 10.8.0 and 10.16.0. +func (v flowVersion) hasLine() bool { return v.atLeast(10, 16) } +func (v flowVersion) hasLineType() bool { return v.atLeast(10, 8) && !v.hasLine() } + +// MicroflowBase.returnVariableName, introduced in 10.12.0. +func (v flowVersion) hasReturnVariableName() bool { return v.atLeast(10, 12) } + +// MicroflowParameterObject.isRequired / defaultValue, introduced in 10.17.0. +func (v flowVersion) hasParameterDefaults() bool { return v.atLeast(10, 17) } + +// flowVersion returns the version this project's flow documents are written for. +func (b *Backend) flowVersion() flowVersion { + pv := b.ProjectVersion() + if pv == nil || pv.MajorVersion == 0 { + return newestFlowVersion + } + return flowVersion{major: pv.MajorVersion, minor: pv.MinorVersion} +} + +// microflowEncoder encodes a Microflows$Microflow for v. StableId comes from a +// registered Studio Pro default (a fresh GUID), not a gen property, so leaving +// it out below its floor is the encoder's job. +func microflowEncoder(v flowVersion) *codec.Encoder { + if v.hasStableID() { + return &codec.Encoder{} + } + return &codec.Encoder{OmitKeys: map[string]map[string]bool{"Microflows$Microflow": {"StableId": true}}} +} + // CreateMicroflow adds a new microflow document (a top-level unit). func (b *Backend) CreateMicroflow(mf *microflows.Microflow) error { if mf == nil { return fmt.Errorf("CreateMicroflow: nil microflow") } + if err := flowImportMappingEntityError(mf.ObjectCollection); err != nil { + return fmt.Errorf("CreateMicroflow %s: %w", mf.Name, err) + } if b.writer == nil { return fmt.Errorf("CreateMicroflow: not connected for writing") } if mf.ID == "" { mf.ID = model.ID(mmpr.GenerateID()) } - gm := microflowToGen(mf, b.majorVersion()) + fv := b.flowVersion() + gm := microflowToGen(mf, fv) gm.SetID(element.ID(mf.ID)) assignMicroflowIDs(gm) - contents, err := (&codec.Encoder{}).Encode(gm) + contents, err := microflowEncoder(fv).Encode(gm) if err != nil { return fmt.Errorf("CreateMicroflow: encode: %w", err) } @@ -172,13 +232,17 @@ func (b *Backend) UpdateMicroflow(mf *microflows.Microflow) error { if mf == nil { return fmt.Errorf("UpdateMicroflow: nil microflow") } + if err := flowImportMappingEntityError(mf.ObjectCollection); err != nil { + return fmt.Errorf("UpdateMicroflow %s: %w", mf.Name, err) + } if b.writer == nil { return fmt.Errorf("UpdateMicroflow: not connected for writing") } - gm := microflowToGen(mf, b.majorVersion()) + fv := b.flowVersion() + gm := microflowToGen(mf, fv) gm.SetID(element.ID(mf.ID)) assignMicroflowIDs(gm) - contents, err := (&codec.Encoder{}).Encode(gm) + contents, err := microflowEncoder(fv).Encode(gm) if err != nil { return fmt.Errorf("UpdateMicroflow: encode: %w", err) } @@ -197,9 +261,10 @@ func (b *Backend) DeleteMicroflow(id model.ID) error { } // microflowToGen builds a gen Microflow wrapper from the model. Mirrors the legacy -// serializer (writer_microflow.go). v10+ adds ReturnVariableName/StableId/Url/ -// UrlSearchParameters. -func microflowToGen(mf *microflows.Microflow, major int) *genMf.Microflow { +// serializer (writer_microflow.go). v10+ adds Url/UrlSearchParameters (the +// property floor drops them empty below 10.1/10.9), 10.12+ ReturnVariableName, +// and 10.2+ StableId (via microflowEncoder). +func microflowToGen(mf *microflows.Microflow, v flowVersion) *genMf.Microflow { out := genMf.NewMicroflow() out.SetName(mf.Name) out.SetDocumentation(mf.Documentation) @@ -239,7 +304,7 @@ func microflowToGen(mf *microflows.Microflow, major int) *genMf.Microflow { // Object collection (parameters merged first, then objects). oc := genMf.NewMicroflowObjectCollection() for i, p := range mf.Parameters { - oc.AddObjects(microflowParameterToGen(p, i, major)) + oc.AddObjects(microflowParameterToGen(p, i, v)) } if mf.ObjectCollection != nil { for _, obj := range mf.ObjectCollection.Objects { @@ -254,22 +319,25 @@ func microflowToGen(mf *microflows.Microflow, major int) *genMf.Microflow { // and annotation flows share the same Flows list. if mf.ObjectCollection != nil { for _, f := range mf.ObjectCollection.Flows { - out.AddFlows(sequenceFlowToGen(f, major)) + out.AddFlows(sequenceFlowToGen(f, v)) } for _, af := range mf.ObjectCollection.AnnotationFlows { - out.AddFlows(annotationFlowToGen(af, major)) + out.AddFlows(annotationFlowToGen(af, v)) } } - if major >= 10 { + if v.hasReturnVariableName() { out.SetReturnVariableName(mf.ReturnVariableName) + } + if v.major >= 10 { // Carried, not hardcoded. These two were `""` and `nil` unconditionally, // so every rewrite deleted the microflow's deep link (#1120). A fresh // microflow has neither, so the empty values still come out empty — // SetUrlSearchParametersQualifiedNames(nil) is the empty marker-1 list. out.SetUrl(mf.URL) // StableId is emitted as a fresh GUID binary via the registered default - // (the gen mistypes it as a string), not set here. + // (the gen mistypes it as a string), not set here; microflowEncoder + // leaves it out below 10.2. out.SetUrlSearchParametersQualifiedNames(mf.URLSearchParameters) } return out @@ -421,7 +489,7 @@ func microflowObjectToGen(obj microflows.MicroflowObject) element.Element { // annotationFlowToGen builds a Microflows$AnnotationFlow connecting an Annotation // (origin) to the activity it documents (destination). The line shape is // version-specific, mirroring the legacy serializeAnnotationFlow. -func annotationFlowToGen(af *microflows.AnnotationFlow, major int) element.Element { +func annotationFlowToGen(af *microflows.AnnotationFlow, v flowVersion) element.Element { g := genMf.NewAnnotationFlow() g.SetID(element.ID(af.ID)) g.SetOriginID(element.ID(af.OriginID)) @@ -431,14 +499,19 @@ func annotationFlowToGen(af *microflows.AnnotationFlow, major int) element.Eleme // drew the line out of the top of the note and back down around it. g.SetOriginConnectionIndex(2) g.SetDestinationConnectionIndex(0) - if major <= 9 { - g.SetOriginBezierVector("0;0") - g.SetDestinationBezierVector("0;0") - } else { + // An annotation flow is a Flow like a sequence flow, with the same floors: + // the Line was written from 10.0 on, six minors before it existed. + if v.hasLine() { line := genMf.NewBezierCurve() line.SetOriginControlVector("0;0") line.SetDestinationControlVector("0;0") g.SetLine(line) + } else { + g.SetOriginBezierVector("0;0") + g.SetDestinationBezierVector("0;0") + if v.hasLineType() { + g.SetLineType("BezierCurve") + } } return g } @@ -1444,7 +1517,7 @@ func memberChangeToGen(m *microflows.MemberChange) element.Element { // microflowParameterToGen builds a gen MicroflowParameter. An authored position // is written as given; without one the position derives from the index, matching // the legacy serializer. -func microflowParameterToGen(p *microflows.MicroflowParameter, idx, major int) element.Element { +func microflowParameterToGen(p *microflows.MicroflowParameter, idx int, v flowVersion) element.Element { g := genMf.NewMicroflowParameter() g.SetID(element.ID(p.ID)) g.SetDocumentation(p.Documentation) @@ -1456,7 +1529,7 @@ func microflowParameterToGen(p *microflows.MicroflowParameter, idx, major int) e } g.SetRelativeMiddlePoint(fmt.Sprintf("%d;%d", pos.X, pos.Y)) g.SetSize("30;30") - if major >= 10 { + if v.hasParameterDefaults() { g.SetDefaultValue("") g.SetIsRequired(true) } @@ -1464,9 +1537,13 @@ func microflowParameterToGen(p *microflows.MicroflowParameter, idx, major int) e return g } -// sequenceFlowToGen builds a gen SequenceFlow. v10+ uses CaseValues + a BezierCurve -// Line; the case defaults to NoCase. -func sequenceFlowToGen(f *microflows.SequenceFlow, major int) element.Element { +// sequenceFlowToGen builds a gen SequenceFlow in the shape v stores; the case +// defaults to NoCase. Measured shapes (mendixlabs/mxcli#1410): +// +// < 10.15 the case as one NewCaseValue document; 10.15+ a CaseValues list +// < 10.16 Origin/DestinationBezierVector strings (with LineType "BezierCurve" +// from 10.8); 10.16+ a BezierCurve Line holding the control vectors +func sequenceFlowToGen(f *microflows.SequenceFlow, v flowVersion) element.Element { g := genMf.NewSequenceFlow() g.SetID(element.ID(f.ID)) g.SetOriginID(element.ID(f.OriginID)) @@ -1474,14 +1551,26 @@ func sequenceFlowToGen(f *microflows.SequenceFlow, major int) element.Element { g.SetOriginConnectionIndex(int32(f.OriginConnectionIndex)) g.SetDestinationConnectionIndex(int32(f.DestinationConnectionIndex)) g.SetIsErrorHandler(f.IsErrorHandler) - g.AddCaseValues(caseValueToGen(f.CaseValue)) + if v.hasCaseValueList() { + g.AddCaseValues(caseValueToGen(f.CaseValue)) + } else { + g.SetCaseValue(caseValueToGen(f.CaseValue)) + } originCV := orDefault(f.OriginControlVector, "0;0") destCV := orDefault(f.DestinationControlVector, "0;0") - line := genMf.NewBezierCurve() - line.SetOriginControlVector(originCV) - line.SetDestinationControlVector(destCV) - g.SetLine(line) + if v.hasLine() { + line := genMf.NewBezierCurve() + line.SetOriginControlVector(originCV) + line.SetDestinationControlVector(destCV) + g.SetLine(line) + } else { + g.SetOriginBezierVector(originCV) + g.SetDestinationBezierVector(destCV) + if v.hasLineType() { + g.SetLineType("BezierCurve") + } + } return g } @@ -1847,14 +1936,21 @@ func assignMicroflowIDs(m *genMf.Microflow) { for _, el := range m.FlowsItems() { assignID(el) if sf, ok := el.(*genMf.SequenceFlow); ok { - for _, cv := range sf.CaseValuesItems() { - assignID(cv) - } - assignID(sf.Line()) + assignSequenceFlowIDs(sf) } } } +// assignSequenceFlowIDs assigns IDs to a sequence flow's case and line, in +// whichever shape the flow was built (sequenceFlowToGen). +func assignSequenceFlowIDs(sf *genMf.SequenceFlow) { + assignID(sf.CaseValue()) + for _, cv := range sf.CaseValuesItems() { + assignID(cv) + } + assignID(sf.Line()) +} + // assignObjectCollectionIDs assigns IDs to a collection's objects and their // sub-elements, recursing into loop bodies. func assignObjectCollectionIDs(oc *genMf.MicroflowObjectCollection) { diff --git a/mdl/backend/modelsdk/nanoflow_carry_test.go b/mdl/backend/modelsdk/nanoflow_carry_test.go index 6c8048f493..ea3a723659 100644 --- a/mdl/backend/modelsdk/nanoflow_carry_test.go +++ b/mdl/backend/modelsdk/nanoflow_carry_test.go @@ -88,7 +88,7 @@ func countFlowType(t *testing.T, raw []byte, typeName string) int { func TestNanoflowToGen_WritesAnnotationFlows(t *testing.T) { nf := µflows.Nanoflow{Name: "NF", ObjectCollection: annotatedCollection()} - g := nanoflowToGen(nf, 11) + g := nanoflowToGen(nf, newestFlowVersion) assignNanoflowIDs(g) raw, err := (&codec.Encoder{}).Encode(g) if err != nil { @@ -105,7 +105,7 @@ func TestNanoflowToGen_WritesAnnotationFlows(t *testing.T) { func TestRuleToGen_WritesAnnotationFlows(t *testing.T) { r := µflows.Rule{Name: "R", ReturnType: µflows.BooleanType{}, ObjectCollection: annotatedCollection()} - g := ruleToGen(r, 11) + g := ruleToGen(r, newestFlowVersion) raw, err := (&codec.Encoder{}).Encode(g) if err != nil { t.Fatalf("encode: %v", err) diff --git a/mdl/backend/modelsdk/nanoflow_write.go b/mdl/backend/modelsdk/nanoflow_write.go index e981e8a4e6..e1972d09f5 100644 --- a/mdl/backend/modelsdk/nanoflow_write.go +++ b/mdl/backend/modelsdk/nanoflow_write.go @@ -29,7 +29,7 @@ func (b *Backend) CreateNanoflow(nf *microflows.Nanoflow) error { if nf.ID == "" { nf.ID = model.ID(mmpr.GenerateID()) } - g := nanoflowToGen(nf, b.majorVersion()) + g := nanoflowToGen(nf, b.flowVersion()) g.SetID(element.ID(nf.ID)) assignNanoflowIDs(g) contents, err := (&codec.Encoder{}).Encode(g) @@ -50,7 +50,7 @@ func (b *Backend) UpdateNanoflow(nf *microflows.Nanoflow) error { if b.writer == nil { return fmt.Errorf("UpdateNanoflow: not connected for writing") } - g := nanoflowToGen(nf, b.majorVersion()) + g := nanoflowToGen(nf, b.flowVersion()) g.SetID(element.ID(nf.ID)) assignNanoflowIDs(g) b.carryStoredNanoflowHeader(nf, g) @@ -76,7 +76,7 @@ func (b *Backend) DeleteNanoflow(id model.ID) error { // serializeNanoflow field set (AllowedModuleRoles, Documentation, Excluded, // Flows, MarkAsUsed, MicroflowReturnType [only when non-void], Name, // ObjectCollection with parameters merged in first). -func nanoflowToGen(nf *microflows.Nanoflow, major int) *genMf.Nanoflow { +func nanoflowToGen(nf *microflows.Nanoflow, v flowVersion) *genMf.Nanoflow { out := genMf.NewNanoflow() out.SetName(nf.Name) out.SetDocumentation(nf.Documentation) @@ -86,14 +86,16 @@ func nanoflowToGen(nf *microflows.Nanoflow, major int) *genMf.Nanoflow { if nf.ReturnType != nil { out.SetMicroflowReturnType(microflowDataTypeToGen(nf.ReturnType)) } - // Only when authored; UpdateNanoflow carries a stored one otherwise. - if nf.ReturnVariableName != "" { + // Only when authored, and only where the version declares it (10.12.0, + // MicroflowBase.returnVariableName); UpdateNanoflow carries a stored one + // otherwise. + if nf.ReturnVariableName != "" && v.hasReturnVariableName() { out.SetReturnVariableName(nf.ReturnVariableName) } oc := genMf.NewMicroflowObjectCollection() for i, p := range nf.Parameters { - oc.AddObjects(microflowParameterToGen(p, i, major)) + oc.AddObjects(microflowParameterToGen(p, i, v)) } if nf.ObjectCollection != nil { for _, obj := range nf.ObjectCollection.Objects { @@ -110,10 +112,10 @@ func nanoflowToGen(nf *microflows.Nanoflow, major int) *genMf.Nanoflow { // (ako/mxcli#705). if nf.ObjectCollection != nil { for _, f := range nf.ObjectCollection.Flows { - out.AddFlows(sequenceFlowToGen(f, major)) + out.AddFlows(sequenceFlowToGen(f, v)) } for _, af := range nf.ObjectCollection.AnnotationFlows { - out.AddFlows(annotationFlowToGen(af, major)) + out.AddFlows(annotationFlowToGen(af, v)) } } return out @@ -172,10 +174,7 @@ func assignNanoflowIDs(n *genMf.Nanoflow) { for _, el := range n.FlowsItems() { assignID(el) if sf, ok := el.(*genMf.SequenceFlow); ok { - for _, cv := range sf.CaseValuesItems() { - assignID(cv) - } - assignID(sf.Line()) + assignSequenceFlowIDs(sf) } } } diff --git a/mdl/backend/modelsdk/rule_write.go b/mdl/backend/modelsdk/rule_write.go index 468439efba..57f837166d 100644 --- a/mdl/backend/modelsdk/rule_write.go +++ b/mdl/backend/modelsdk/rule_write.go @@ -37,7 +37,7 @@ func (b *Backend) CreateRule(rule *microflows.Rule) error { if rule.ID == "" { rule.ID = model.ID(mmpr.GenerateID()) } - g := ruleToGen(rule, b.majorVersion()) + g := ruleToGen(rule, b.flowVersion()) g.SetID(element.ID(rule.ID)) assignRuleIDs(g) contents, err := (&codec.Encoder{}).Encode(g) @@ -58,7 +58,7 @@ func (b *Backend) UpdateRule(rule *microflows.Rule) error { if b.writer == nil { return fmt.Errorf("UpdateRule: not connected for writing") } - g := ruleToGen(rule, b.majorVersion()) + g := ruleToGen(rule, b.flowVersion()) g.SetID(element.ID(rule.ID)) assignRuleIDs(g) contents, err := (&codec.Encoder{}).Encode(g) @@ -96,7 +96,7 @@ func (b *Backend) DeleteRule(id model.ID) error { // pre-7 legacy sibling with nothing to carry through. // - No concurrency group, Url, StableId or action-info slots: those are the // nine properties that make a microflow a microflow. -func ruleToGen(rule *microflows.Rule, major int) *genMf.Rule { +func ruleToGen(rule *microflows.Rule, v flowVersion) *genMf.Rule { out := genMf.NewRule() out.SetName(rule.Name) out.SetDocumentation(rule.Documentation) @@ -111,14 +111,16 @@ func ruleToGen(rule *microflows.Rule, major int) *genMf.Rule { out.SetExcluded(rule.Excluded) out.SetMarkAsUsed(rule.MarkAsUsed) out.SetApplyEntityAccess(rule.ApplyEntityAccess) - out.SetReturnVariableName(rule.ReturnVariableName) + if v.hasReturnVariableName() { // MicroflowBase.returnVariableName, 10.12.0 + out.SetReturnVariableName(rule.ReturnVariableName) + } if rule.ReturnType != nil { out.SetMicroflowReturnType(microflowDataTypeToGen(rule.ReturnType)) } oc := genMf.NewMicroflowObjectCollection() for i, p := range rule.Parameters { - oc.AddObjects(microflowParameterToGen(p, i, major)) + oc.AddObjects(microflowParameterToGen(p, i, v)) } if rule.ObjectCollection != nil { for _, obj := range rule.ObjectCollection.Objects { @@ -131,12 +133,12 @@ func ruleToGen(rule *microflows.Rule, major int) *genMf.Rule { if rule.ObjectCollection != nil { for _, f := range rule.ObjectCollection.Flows { - out.AddFlows(sequenceFlowToGen(f, major)) + out.AddFlows(sequenceFlowToGen(f, v)) } // Omitted like the nanoflow's, and for the same effect: a rewrite // detached every note (ako/mxcli#705). for _, af := range rule.ObjectCollection.AnnotationFlows { - out.AddFlows(annotationFlowToGen(af, major)) + out.AddFlows(annotationFlowToGen(af, v)) } } return out @@ -154,10 +156,7 @@ func assignRuleIDs(r *genMf.Rule) { for _, el := range r.FlowsItems() { assignID(el) if sf, ok := el.(*genMf.SequenceFlow); ok { - for _, cv := range sf.CaseValuesItems() { - assignID(cv) - } - assignID(sf.Line()) + assignSequenceFlowIDs(sf) } } } diff --git a/mdl/backend/modelsdk/rule_write_test.go b/mdl/backend/modelsdk/rule_write_test.go index 4170ba32bf..c751dbeae9 100644 --- a/mdl/backend/modelsdk/rule_write_test.go +++ b/mdl/backend/modelsdk/rule_write_test.go @@ -31,7 +31,7 @@ func TestAuthoredRuleMatchesStudioProKeySet(t *testing.T) { ReturnVariableName: "Variable", } - encoded, err := (&codec.Encoder{}).Encode(ruleToGen(rule, 11)) + encoded, err := (&codec.Encoder{}).Encode(ruleToGen(rule, newestFlowVersion)) if err != nil { t.Fatalf("encode rule: %v", err) } @@ -64,7 +64,7 @@ func TestAuthoredRuleSetsExportLevel(t *testing.T) { encoded, err := (&codec.Encoder{}).Encode(ruleToGen(µflows.Rule{ Name: "R", ReturnType: µflows.BooleanType{}, - }, 11)) + }, newestFlowVersion)) if err != nil { t.Fatalf("encode rule: %v", err) } diff --git a/mdl/executor/cmd_flow_modify.go b/mdl/executor/cmd_flow_modify.go index 756b66ddcd..0d96cb32d2 100644 --- a/mdl/executor/cmd_flow_modify.go +++ b/mdl/executor/cmd_flow_modify.go @@ -237,6 +237,17 @@ func planFlowModify(ctx *ExecContext, d *flowDecl) (*flowPlan, error) { } return nil, cannotSplice("the stored %s cannot be read for splicing: %v", d.kind(), err) } + // A stored import activity without its result entity (what mxcli wrote + // before mendixlabs/mxcli#1418 for a mapping created further down the + // script) makes the project unloadable, and describe prints it exactly as + // the statement that would repair it, so the statement diff finds nothing + // and reported "Unchanged" over a broken project. Treat it as a change the + // splice cannot make: mdl 0 rebuilds the flow — resolving the mapping + // afresh — and mdl 1 refuses with the drop-and-create advice, rather than + // keeping it silently. + if mapping := storedImportWithoutEntity(a.mf.ObjectCollection); mapping != "" { + return nil, cannotSplice("its stored `import from mapping %s` activity has no result entity, which Mendix cannot load (mendixlabs/mxcli#1418)", mapping) + } stored, err := describedFlowStmt(ctx, d, a) if err != nil { @@ -378,6 +389,32 @@ func fallBack(ctx *ExecContext, d *flowDecl, v flowVerdict) (bool, error) { return false, nil } +// storedImportWithoutEntity returns the mapping of the first import-from-mapping +// activity in oc (loop bodies included) whose result entity is empty, or "". +func storedImportWithoutEntity(oc *microflows.MicroflowObjectCollection) string { + if oc == nil { + return "" + } + for _, obj := range oc.Objects { + switch o := obj.(type) { + case *microflows.ActionActivity: + if ix, ok := o.Action.(*microflows.ImportXmlAction); ok { + if ix.ResultHandling == nil { + return "(none)" + } + if ix.ResultHandling.ResultEntityID == "" { + return string(ix.ResultHandling.MappingID) + } + } + case *microflows.LoopedActivity: + if m := storedImportWithoutEntity(o.ObjectCollection); m != "" { + return m + } + } + } + return "" +} + // reportUnchanged reports a statement that wrote nothing, collapsing into the // run's summary like an elided write does. func reportUnchanged(ctx *ExecContext, what string) { diff --git a/mdl/executor/cmd_javaactions.go b/mdl/executor/cmd_javaactions.go index 0aa66181fe..ceba363665 100644 --- a/mdl/executor/cmd_javaactions.go +++ b/mdl/executor/cmd_javaactions.go @@ -105,9 +105,6 @@ func describeJavaAction(ctx *ExecContext, name ast.QualifiedName) error { } for i, param := range ja.Parameters { - if i > 0 { - sb.WriteString(", ") - } if hasParamDescriptions { sb.WriteString("\n ") } @@ -121,6 +118,14 @@ func describeJavaAction(ctx *ExecContext, name ast.QualifiedName) error { if param.IsRequired { sb.WriteString(" not null") } + // The separator precedes any `--` doc comment: written after it, the + // comment swallows the comma and the next parameter fails to parse (#1412). + if i < len(ja.Parameters)-1 { + sb.WriteString(",") + if !hasParamDescriptions { + sb.WriteString(" ") + } + } if param.Description != "" { paramDoc := strings.ReplaceAll(param.Description, "\r\n", "\n") paramDoc = strings.ReplaceAll(paramDoc, "\r", "\n") diff --git a/mdl/executor/cmd_javascript_actions.go b/mdl/executor/cmd_javascript_actions.go index c771659cf7..db1f07d9c4 100644 --- a/mdl/executor/cmd_javascript_actions.go +++ b/mdl/executor/cmd_javascript_actions.go @@ -118,9 +118,6 @@ func describeJavaScriptAction(ctx *ExecContext, name ast.QualifiedName) error { } for i, param := range jsa.Parameters { - if i > 0 { - sb.WriteString(", ") - } if hasParamDescriptions { sb.WriteString("\n ") } @@ -134,6 +131,14 @@ func describeJavaScriptAction(ctx *ExecContext, name ast.QualifiedName) error { if param.IsRequired { sb.WriteString(" not null") } + // The separator precedes any `--` doc comment: written after it, the + // comment swallows the comma and the next parameter fails to parse (#1412). + if i < len(jsa.Parameters)-1 { + sb.WriteString(",") + if !hasParamDescriptions { + sb.WriteString(" ") + } + } if param.Description != "" { paramDoc := strings.ReplaceAll(param.Description, "\r\n", "\n") paramDoc = strings.ReplaceAll(paramDoc, "\r", "\n") diff --git a/mdl/executor/cmd_microflows_builder.go b/mdl/executor/cmd_microflows_builder.go index 10149a0335..89bcc35c2b 100644 --- a/mdl/executor/cmd_microflows_builder.go +++ b/mdl/executor/cmd_microflows_builder.go @@ -225,6 +225,29 @@ func (fb *flowBuilder) hasDeclaredReturnValue() bool { return fb.returnType != nil && fb.returnType.Type.Kind != ast.TypeVoid } +// addUndeclaredSetError reports `set $X = …` on a variable that is not +// declared. When $X is the flow's `returns … as $X` name the generic message +// reads as contradicting that clause, so it says what the clause does: it names +// the result for callers and declares nothing (mendixlabs/mxcli#1436). +func (fb *flowBuilder) addUndeclaredSetError(target string) { + name := strings.TrimPrefix(target, "$") + if fb.returnType != nil && fb.returnType.Variable != "" && + strings.TrimPrefix(fb.returnType.Variable, "$") == name { + typ := fb.returnType.Type.Kind.String() + if ref := fb.returnType.Type.EntityRef; ref != nil { + typ = ref.Module + "." + ref.Name + } + fb.addErrorWithExample( + fmt.Sprintf("variable '%s' is not declared: `returns … as $%s` names the result for callers "+ + "and does not declare $%s — declare it before the first `set`", name, name, name), + fmt.Sprintf(" declare $%s %s = …;\n ...\n set $%s = …;", name, typ, name)) + return + } + fb.addErrorWithExample( + fmt.Sprintf("variable '%s' is not declared", target), + errorExampleDeclareVariable(target)) +} + // errorExampleDeclareVariable returns an example for declaring a variable. func errorExampleDeclareVariable(varName string) string { // Remove $ prefix if present for cleaner display diff --git a/mdl/executor/cmd_microflows_builder_actions.go b/mdl/executor/cmd_microflows_builder_actions.go index 172e6d0e5c..a51e5db312 100644 --- a/mdl/executor/cmd_microflows_builder_actions.go +++ b/mdl/executor/cmd_microflows_builder_actions.go @@ -67,9 +67,7 @@ func (fb *flowBuilder) addCreateVariableAction(s *ast.DeclareStmt) model.ID { func (fb *flowBuilder) addChangeVariableAction(s *ast.MfSetStmt) model.ID { // Validate that the variable has been declared if !fb.isVariableDeclared(s.Target) { - fb.addErrorWithExample( - fmt.Sprintf("variable '%s' is not declared", s.Target), - errorExampleDeclareVariable(s.Target)) + fb.addUndeclaredSetError(s.Target) } activityX := fb.posX @@ -1775,10 +1773,16 @@ func (fb *flowBuilder) addListOperationAction(s *ast.ListOperationStmt) model.ID if !spec.Ascending { direction = microflows.SortDirectionDescending } - // Build fully qualified attribute name: Entity.Attribute + // Qualify with the entity that DECLARES the attribute, as the + // retrieve sort does: an inherited name qualified with the list's + // own entity is CE1613 (mendixlabs/mxcli#1430). attrQN := spec.Attribute if entityType != "" && !strings.Contains(spec.Attribute, ".") { - attrQN = entityType + "." + spec.Attribute + if declared, ok := fb.resolveAttributeInEntityHierarchy(entityType, spec.Attribute); ok { + attrQN = declared + } else { + attrQN = entityType + "." + spec.Attribute + } } if fb.qualifiedMembersOnly { fb.refuseUnqualifiedAttribute(attrQN, spec.Attribute, entityType) @@ -2012,6 +2016,26 @@ func (fb *flowBuilder) addAggregateListAction(s *ast.AggregateListStmt) model.ID } } + // Declare the result so a later SET on it is accepted, as the validate path + // already does (mendixlabs/mxcli#1413). SUM/MIN/MAX follow the attribute's + // type, which is not resolved here, so they stay Unknown. + if fb.declaredVars != nil && s.OutputVariable != "" { + resultType := "Unknown" + switch s.Operation { + case ast.AggregateCount: + resultType = "Integer" + case ast.AggregateAverage: + resultType = "Decimal" + case ast.AggregateAll, ast.AggregateAny: + resultType = "Boolean" + case ast.AggregateReduce: + if s.ReturnType != nil { + resultType = s.ReturnType.Kind.String() + } + } + fb.declaredVars[s.OutputVariable] = resultType + } + activity := µflows.ActionActivity{ BaseActivity: microflows.BaseActivity{ BaseMicroflowObject: microflows.BaseMicroflowObject{ diff --git a/mdl/executor/cmd_microflows_builder_aggregate_declared_test.go b/mdl/executor/cmd_microflows_builder_aggregate_declared_test.go new file mode 100644 index 0000000000..9c7aa0dca4 --- /dev/null +++ b/mdl/executor/cmd_microflows_builder_aggregate_declared_test.go @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// mendixlabs/mxcli#1413: `$Sum = sum $List by Amount; set $Sum = $Sum + 1;` +// passed `check -p` but `exec` refused it with "variable 'Sum' is not +// declared" — the build path never registered an aggregate's output variable, +// while the validate path did. +func TestAggregateOutputIsDeclaredForSet(t *testing.T) { + cases := []struct { + name string + stmt *ast.AggregateListStmt + want string // declaredVars type, "" = only require it be declared + }{ + {"sum", &ast.AggregateListStmt{OutputVariable: "Sum", Operation: ast.AggregateSum, InputVariable: "List", Attribute: "Amount"}, ""}, + {"count", &ast.AggregateListStmt{OutputVariable: "Sum", Operation: ast.AggregateCount, InputVariable: "List"}, "Integer"}, + {"average", &ast.AggregateListStmt{OutputVariable: "Sum", Operation: ast.AggregateAverage, InputVariable: "List", Attribute: "Amount"}, "Decimal"}, + {"maximum", &ast.AggregateListStmt{OutputVariable: "Sum", Operation: ast.AggregateMaximum, InputVariable: "List", Attribute: "Amount"}, ""}, + {"any", &ast.AggregateListStmt{OutputVariable: "Sum", Operation: ast.AggregateAny, InputVariable: "List", + IsExpression: true, Expression: &ast.LiteralExpr{Kind: ast.LiteralBoolean, Value: true}}, "Boolean"}, + {"reduce", &ast.AggregateListStmt{OutputVariable: "Sum", Operation: ast.AggregateReduce, InputVariable: "List", + IsExpression: true, Expression: &ast.VariableExpr{Name: "currentResult"}, + InitialValue: &ast.LiteralExpr{Kind: ast.LiteralInteger, Value: int64(0)}, + ReturnType: &ast.DataType{Kind: ast.TypeInteger}}, "Integer"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + body := []ast.MicroflowStatement{ + tc.stmt, + &ast.MfSetStmt{Target: "Sum", Value: &ast.BinaryExpr{ + Left: &ast.VariableExpr{Name: "Sum"}, + Operator: "+", + Right: &ast.LiteralExpr{Kind: ast.LiteralInteger, Value: int64(1)}, + }}, + } + fb := &flowBuilder{ + posX: 200, posY: 200, baseY: 200, spacing: HorizontalSpacing, + varTypes: map[string]string{"List": "List of ModA.EntA"}, + declaredVars: map[string]string{}, + } + fb.buildFlowGraph(body, nil) + if errs := fb.GetErrors(); len(errs) > 0 { + t.Fatalf("build: %s", strings.Join(errs, "; ")) + } + if tc.want != "" && fb.declaredVars["Sum"] != tc.want { + t.Errorf("declared type = %q, want %q", fb.declaredVars["Sum"], tc.want) + } + }) + } +} diff --git a/mdl/executor/cmd_microflows_builder_calls.go b/mdl/executor/cmd_microflows_builder_calls.go index dff6a7f7d0..2eb033263c 100644 --- a/mdl/executor/cmd_microflows_builder_calls.go +++ b/mdl/executor/cmd_microflows_builder_calls.go @@ -2096,15 +2096,29 @@ func (fb *flowBuilder) addImportFromMappingAction(s *ast.ImportFromMappingStmt) // JSON structure check covers JSON-backed mappings; for XML schema or // message-definition mappings JsonStructure is empty and the root // element kind on the mapping itself indicates Array vs Object. + // + // A mapping that does not resolve is refused, not written around. The + // result entity is stored as the activity's VariableType.Entity, and an + // empty one makes the whole project unloadable — `mx check` dies with + // ArgumentNullException in EntityType.set_EntityId before validating + // anything (mendixlabs/mxcli#1418). The usual cause is a script that calls + // the mapping above its `create import mapping`; MDL-ORDER01 reports that + // at check time, this is the exec-time half. resultEntityQN := "" if fb.backend != nil { - if im, err := fb.backend.GetImportMappingByQualifiedName(s.Mapping.Module, s.Mapping.Name); err == nil { + im, err := fb.backend.GetImportMappingByQualifiedName(s.Mapping.Module, s.Mapping.Name) + switch { + case err != nil || im == nil: + fb.addError("import from mapping %s: import mapping not found in the project (create it above this microflow; an activity without its result entity makes the project unloadable)", s.Mapping.String()) + default: if !rangeAuthored && fb.mappingRootIsList(im) { resultHandling.SingleObject = false } if len(im.Elements) > 0 && im.Elements[0] != nil && im.Elements[0].Entity != "" { resultEntityQN = im.Elements[0].Entity resultHandling.ResultEntityID = model.ID(resultEntityQN) + } else { + fb.addError("import from mapping %s: the import mapping has no root entity, so the activity's result type cannot be written (an empty one makes the project unloadable)", s.Mapping.String()) } } } diff --git a/mdl/executor/cmd_microflows_builder_validate.go b/mdl/executor/cmd_microflows_builder_validate.go index e727739456..179e87322c 100644 --- a/mdl/executor/cmd_microflows_builder_validate.go +++ b/mdl/executor/cmd_microflows_builder_validate.go @@ -13,13 +13,13 @@ import ( // ValidateMicroflowBody validates the microflow body for semantic errors without building objects. // This is used by the check command to validate scripts without executing them. func ValidateMicroflowBody(s *ast.CreateMicroflowStmt) []string { - return validateFlowBody(s.Parameters, s.Body, true, nil) + return validateFlowBody(s.Parameters, s.ReturnType, s.Body, true, nil) } // ValidateNanoflowBody validates the nanoflow body for semantic errors without building objects. // This is used by the check command to validate scripts without executing them. func ValidateNanoflowBody(s *ast.CreateNanoflowStmt) []string { - return validateFlowBody(s.Parameters, s.Body, true, nil) + return validateFlowBody(s.Parameters, s.ReturnType, s.Body, true, nil) } // validateFlowBody validates parameters and body statements for semantic errors. @@ -27,7 +27,7 @@ func ValidateNanoflowBody(s *ast.CreateNanoflowStmt) []string { // flowBuilder.duplicateNamesOwnedElsewhere. assocs, when check --references has // a project, types an association retrieve as the object or list it yields; // nil leaves every association retrieve a list, as without a project. -func validateFlowBody(params []ast.MicroflowParam, body []ast.MicroflowStatement, duplicatesOwnedElsewhere bool, assocs map[string]assocShape) []string { +func validateFlowBody(params []ast.MicroflowParam, returns *ast.MicroflowReturnType, body []ast.MicroflowStatement, duplicatesOwnedElsewhere bool, assocs map[string]assocShape) []string { varTypes := make(map[string]string) declaredVars := make(map[string]string) @@ -57,6 +57,7 @@ func validateFlowBody(params []ast.MicroflowParam, body []ast.MicroflowStatement fb := &flowBuilder{ varTypes: varTypes, declaredVars: declaredVars, + returnType: returns, // only for messages (addUndeclaredSetError) errors: []string{}, duplicateNamesOwnedElsewhere: duplicatesOwnedElsewhere, checkAssocShapes: assocs, @@ -104,9 +105,7 @@ func (fb *flowBuilder) validateStatement(stmt ast.MicroflowStatement) { case *ast.MfSetStmt: // Validate that the variable has been declared if !fb.isVariableDeclared(s.Target) { - fb.addErrorWithExample( - fmt.Sprintf("variable '%s' is not declared", s.Target), - errorExampleDeclareVariable(s.Target)) + fb.addUndeclaredSetError(s.Target) } else if fb.assocObjectVars[strings.TrimPrefix(s.Target, "$")] { // Only the association-retrieved object: every other object // producer is visible without a project, and MDL-SET01 reports @@ -466,7 +465,7 @@ func (fb *flowBuilder) validateOutputVariable(varName, statement string) { // counterpart of ValidateMicroflowBody. What a rule may not *contain* is a // separate question, answered by validateRule. func ValidateRuleBody(s *ast.CreateRuleStmt) []string { - return validateFlowBody(s.Parameters, s.Body, false, nil) + return validateFlowBody(s.Parameters, nil, s.Body, false, nil) } // forwardReferenceTarget reports the entity a retrieve over association yields diff --git a/mdl/executor/cmd_microflows_list_sort_inherited_attribute_test.go b/mdl/executor/cmd_microflows_list_sort_inherited_attribute_test.go new file mode 100644 index 0000000000..01841f0e88 --- /dev/null +++ b/mdl/executor/cmd_microflows_list_sort_inherited_attribute_test.go @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// mendixlabs/mxcli#1430: the list-operation `sort $L by Attr` qualified an +// INHERITED attribute against the list's own entity, which mxbuild rejects: +// +// CE1613 "The selected attribute 'Embeddings_SemanticSearch.BookChunk.Similarity' +// no longer exists." +// +// The retrieve `sort by` path was fixed for the same class (CapTrackV2 §13) via +// resolveAttributeInEntityHierarchy; the list-operation path still prefixed +// the list entity blindly. Reuses inheritedSortBackend: AppUser extends +// SyntheticBase.User, which declares Name; AppUser declares FullName itself. +func listSortAttrOf(t *testing.T, attr string) []string { + t.Helper() + fb := &flowBuilder{ + backend: inheritedSortBackend(), + spacing: 100, + varTypes: map[string]string{"Users": "List of SyntheticApp.AppUser"}, + } + fb.addListOperationAction(&ast.ListOperationStmt{ + OutputVariable: "Sorted", + Operation: ast.ListOpSort, + InputVariable: "Users", + SortSpecs: []ast.SortSpec{{Attribute: attr, Ascending: false}}, + }) + if len(fb.errors) > 0 { + t.Fatalf("unexpected errors: %v", fb.errors) + } + var paths []string + for _, obj := range fb.objects { + act, ok := obj.(*microflows.ActionActivity) + if !ok { + continue + } + lo, ok := act.Action.(*microflows.ListOperationAction) + if !ok { + continue + } + if so, ok := lo.Operation.(*microflows.SortOperation); ok { + for _, s := range so.Sorting { + paths = append(paths, s.AttributeQualifiedName) + } + } + } + return paths +} + +func TestListSort_InheritedAttributeUsesTheDeclaringEntity(t *testing.T) { + paths := listSortAttrOf(t, "Name") + if len(paths) != 1 || paths[0] != "SyntheticBase.User.Name" { + t.Errorf("stored %v, want [SyntheticBase.User.Name] — the list's own entity "+ + "does not declare Name, and mxbuild rejects that reference with CE1613", paths) + } +} + +// CONTROL: an attribute the entity declares itself is still qualified with it. +func TestListSort_OwnAttributeIsUnchanged(t *testing.T) { + paths := listSortAttrOf(t, "FullName") + if len(paths) != 1 || paths[0] != "SyntheticApp.AppUser.FullName" { + t.Errorf("stored %v, want [SyntheticApp.AppUser.FullName]", paths) + } +} diff --git a/mdl/executor/cmd_pages_describe_childslots.go b/mdl/executor/cmd_pages_describe_childslots.go index 6cbd03c548..4c64a87615 100644 --- a/mdl/executor/cmd_pages_describe_childslots.go +++ b/mdl/executor/cmd_pages_describe_childslots.go @@ -13,7 +13,8 @@ import ( // items. type rawChildSlot struct { // Keyword is the MDL container keyword, derived from the property key the - // same way an object list's is (tagContentContainer -> tagcontentcontainer). + // same way the widget definition derives it (mdlContainerForWidgetSlot: + // tagContentContainer -> tagcontentcontainer, content -> template). Keyword string // PropertyKey is the stored key, kept so the emitted MDL can be matched back // to the document when debugging. @@ -56,6 +57,8 @@ func extractChildSlots(ctx *ExecContext, w map[string]any, entityContext string) return nil } + widgetID := extractCustomWidgetID(ctx, w) + var out []rawChildSlot for _, prop := range getBsonArrayElements(obj["Properties"]) { propMap, ok := prop.(map[string]any) @@ -87,7 +90,11 @@ func extractChildSlots(ctx *ExecContext, w map[string]any, entityContext string) continue } - kw := strings.ToLower(deriveObjectListKeyword(key)) + // The keyword the widget definition declares for this slot, so exec + // reads the describe back: a `content` slot is `template` there + // (mendixlabs/mxcli#1400). deriveObjectListKeyword is the OBJECT-LIST + // convention and named it `content`. + kw := strings.ToLower(mdlContainerForWidgetSlot(widgetID, key)) if kw == "" { kw = strings.ToLower(key) } diff --git a/mdl/executor/cmd_pages_describe_containers_test.go b/mdl/executor/cmd_pages_describe_containers_test.go index 1cdcd5f884..2a4b55f663 100644 --- a/mdl/executor/cmd_pages_describe_containers_test.go +++ b/mdl/executor/cmd_pages_describe_containers_test.go @@ -145,3 +145,54 @@ func TestExtractChildSlots_NoContainers(t *testing.T) { t.Errorf("got %+v, want none", got) } } + +// A child slot is described under the keyword its widget definition declares +// for it, or exec cannot read the describe back. A native Repeater's (and +// SafeAreaView's) `content` slot is `template` on the definition side +// (mdlContainerForWidgetSlot); describe derived `content` from the bare key, +// and check refused it: "`content` is not a container of `repeater` — it +// declares: template [MDL-WIDGET26]" (mendixlabs/mxcli#1400). The +// tagContentContainer slot is the control: a key without an override is +// described as itself, as before. +func TestExtractChildSlots_KeywordMatchesDefinition(t *testing.T) { + w := map[string]any{ + "Type": map[string]any{ + "WidgetId": "com.mendix.widget.native.repeater.Repeater", + "ObjectType": map[string]any{ + "PropertyTypes": []any{ + int32(3), + map[string]any{"$ID": "t1", "PropertyKey": "content"}, + map[string]any{"$ID": "t2", "PropertyKey": "tagContentContainer"}, + }, + }, + }, + "Object": map[string]any{ + "Properties": []any{ + int32(3), + map[string]any{ + "TypePointer": "t1", + "Value": map[string]any{"Widgets": []any{int32(3), + map[string]any{"$Type": "Forms$DynamicText", "Name": "a"}}}, + }, + map[string]any{ + "TypePointer": "t2", + "Value": map[string]any{"Widgets": []any{int32(3), + map[string]any{"$Type": "Forms$DynamicText", "Name": "b"}}}, + }, + }, + }, + } + + got := map[string]string{} + for _, s := range extractChildSlots(&ExecContext{}, w, "") { + got[s.PropertyKey] = s.Keyword + } + for key, want := range map[string]string{ + "content": "template", + "tagContentContainer": "tagcontentcontainer", + } { + if got[key] != want { + t.Errorf("slot %q described as %q, want %q (the keyword the definition declares)", key, got[key], want) + } + } +} diff --git a/mdl/executor/cmd_security.go b/mdl/executor/cmd_security.go index 0dec8a201d..1edb2cee69 100644 --- a/mdl/executor/cmd_security.go +++ b/mdl/executor/cmd_security.go @@ -870,8 +870,7 @@ func describeUserRole(ctx *ExecContext, name ast.QualifiedName) error { // replaces had no slot for the description or the check-security flag, which // describe printed as `--` comments that a replay dropped, and it could not // write a role with no module roles at all. A property at Mendix's default for -// a new role is left out (R12), so an empty role is `create or modify user -// role R;`. +// a new role is left out (R12), except CheckSecurity, which is always stated. func formatUserRoleMDL(ur *security.UserRole) string { var props []string if len(ur.ModuleRoles) > 0 { @@ -889,9 +888,10 @@ func formatUserRoleMDL(ur *security.UserRole) string { if ur.ManageUsersWithoutRoles { props = append(props, "ManageUsersWithoutRoles: true") } - if ur.CheckSecurity { - props = append(props, "CheckSecurity: true") - } + // Always stated: a new role is written with CheckSecurity true, so leaving + // out a false value re-enabled security checking on replay, and `create or + // modify` keeps an unstated stored value either way (#1427). + props = append(props, fmt.Sprintf("CheckSecurity: %t", ur.CheckSecurity)) head := "create or modify user role " + ur.Name switch len(props) { case 0: diff --git a/mdl/executor/cmd_settings.go b/mdl/executor/cmd_settings.go index 8f530c071c..73db42a39d 100644 --- a/mdl/executor/cmd_settings.go +++ b/mdl/executor/cmd_settings.go @@ -359,12 +359,24 @@ func storedModelSettings(ps *model.ProjectSettings) map[string]any { // property the type may not define), so without this check the statement would // report success and change nothing — the silent no-op shape of #805. // -// JavaVersion is exempt: it is stored under either JavaVersion or -// JavaMajorVersion, and settingsoverlay.JavaVersionKey resolves which. +// JavaVersion is stored under either JavaVersion or JavaMajorVersion, and +// settingsoverlay.JavaVersionKey resolves which. A document carrying neither +// (Mendix 9.x) has no Java version setting at all, and the overlay would write +// nothing — mendixlabs/mxcli#1438 reported "Unchanged model settings" there. func refuseIfNotStored(raw map[string]any, key string) error { - if raw == nil || key == "JavaVersion" { + if raw == nil { return nil } + if key == "JavaVersion" { + if settingsoverlay.JavaVersionKey(raw) != "" { + return nil + } + return mdlerrors.NewUnsupported( + "this project does not store the model setting JavaVersion\n" + + " The JavaVersion setting does not exist on this project's Mendix version " + + "(Mendix 9.x has none); the JDK is chosen by the platform.\n" + + " hint: upgrade the project to choose a Java version in the model") + } if settingsoverlay.Has(raw, key) { return nil } diff --git a/mdl/executor/cmd_settings_model_test.go b/mdl/executor/cmd_settings_model_test.go index 1ffa36dcaa..eba447f8dd 100644 --- a/mdl/executor/cmd_settings_model_test.go +++ b/mdl/executor/cmd_settings_model_test.go @@ -276,6 +276,49 @@ func TestAlterSettingsModel_RefusesPropertyThisVersionDoesNotStore(t *testing.T) } } +// TestAlterSettingsModel_JavaVersionRefusedWhenNotStored is mendixlabs/mxcli#1438: +// a Mendix 9.x document stores neither JavaVersion nor JavaMajorVersion, so the +// overlay has no key to write and `alter settings runtime (JavaVersion: '17')` +// reported "Unchanged model settings". It must refuse instead. +func TestAlterSettingsModel_JavaVersionRefusedWhenNotStored(t *testing.T) { + var written *model.ProjectSettings + // A Mendix 9.24-shaped document: no Java version key at all. + ctx, _ := newMockCtx(t, withBackend(modelSettingsBackend(&written, "HashAlgorithm", "BcryptCost"))) + + err := alterSettings(ctx, &ast.AlterSettingsStmt{ + Section: "model", + Properties: map[string]any{"JavaVersion": "17"}, + }) + if err == nil { + t.Fatal("expected a refusal: this project stores no JavaVersion setting, got nil (silent no-op)") + } + if !strings.Contains(err.Error(), "JavaVersion") { + t.Errorf("error should name the property, got: %v", err) + } + if written != nil { + t.Error("a refused statement must not write") + } +} + +// The JavaVersion exemption still applies when either storage key is present. +func TestAlterSettingsModel_JavaVersionAllowedUnderEitherKey(t *testing.T) { + for _, key := range []string{"JavaVersion", "JavaMajorVersion"} { + t.Run(key, func(t *testing.T) { + var written *model.ProjectSettings + ctx, _ := newMockCtx(t, withBackend(modelSettingsBackend(&written, key))) + if err := alterSettings(ctx, &ast.AlterSettingsStmt{ + Section: "model", + Properties: map[string]any{"JavaVersion": "21"}, + }); err != nil { + t.Fatalf("alterSettings: %v", err) + } + if written == nil { + t.Fatal("no settings written") + } + }) + } +} + // The gate must not block a property the document does carry. func TestAlterSettingsModel_AllowsStoredProperty(t *testing.T) { var written *model.ProjectSettings diff --git a/mdl/executor/describe_code_action_param_separator_test.go b/mdl/executor/describe_code_action_param_separator_test.go new file mode 100644 index 0000000000..452d68aa94 --- /dev/null +++ b/mdl/executor/describe_code_action_param_separator_test.go @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/javaactions" +) + +// documentedParams has a description on non-final parameters: describe +// prints each as a trailing `--` comment, so the separator must come before it. +func documentedParams() []*types.JavaActionParameter { + return []*types.JavaActionParameter{ + {Name: "Name", Description: "This sets the Name of the tool.", IsRequired: true, ParameterType: &types.StringType{}}, + {Name: "Amount", Description: "How many.", ParameterType: &types.IntegerType{}}, + {Name: "Extra", ParameterType: &types.StringType{}}, + } +} + +// mendixlabs/mxcli#1412: the separator was written at the start of the next +// parameter, i.e. after the previous one's `-- doc` comment, so the comment +// swallowed the comma and the next parameter failed to parse. +func TestDescribeJavaAction_ParamDocCommentKeepsSeparator(t *testing.T) { + mod := mkModule("MyModule") + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ReadJavaActionByNameFunc: func(string) (*javaactions.JavaAction, error) { + return &javaactions.JavaAction{ + BaseElement: model.BaseElement{ID: nextID("ja")}, + ContainerID: mod.ID, + Name: "CreateTool", + Parameters: documentedParams(), + }, nil + }, + } + ctx, buf := newMockCtx(t, withBackend(mb)) + assertNoError(t, describeJavaAction(ctx, ast.QualifiedName{Module: "MyModule", Name: "CreateTool"})) + out := buf.String() + stmt := findStmt[*ast.CreateJavaActionStmt](t, reparse(t, out), out) + if len(stmt.Parameters) != 3 { + t.Fatalf("re-parsed %d parameters, want 3\n%s", len(stmt.Parameters), out) + } +} + +func TestDescribeJavaScriptAction_ParamDocCommentKeepsSeparator(t *testing.T) { + mod := mkModule("WebMod") + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ReadJavaScriptActionByNameFunc: func(string) (*types.JavaScriptAction, error) { + return &types.JavaScriptAction{ + BaseElement: model.BaseElement{ID: nextID("jsa")}, + ContainerID: mod.ID, + Name: "CreateTool", + Platform: "Web", + Parameters: documentedParams(), + }, nil + }, + } + ctx, buf := newMockCtx(t, withBackend(mb)) + assertNoError(t, describeJavaScriptAction(ctx, ast.QualifiedName{Module: "WebMod", Name: "CreateTool"})) + out := buf.String() + stmt := findStmt[*ast.CreateJavaScriptActionStmt](t, reparse(t, out), out) + if len(stmt.Parameters) != 3 { + t.Fatalf("re-parsed %d parameters, want 3\n%s", len(stmt.Parameters), out) + } +} diff --git a/mdl/executor/executor.go b/mdl/executor/executor.go index 7b71fee34e..f7c389d470 100644 --- a/mdl/executor/executor.go +++ b/mdl/executor/executor.go @@ -417,6 +417,13 @@ func (e *Executor) Execute(stmt ast.Statement) error { } } + // A handler that ignored the guard's Write error still "succeeded" with its + // output cut off at the limit; that must fail the statement, not exit 0 + // (mendixlabs/mxcli#1425). + if err == nil && e.guard != nil { + err = e.guard.exceededError() + } + if e.logger != nil { e.logger.Command(stmtTypeName(stmt), stmtSummary(stmt), time.Since(start), err) } diff --git a/mdl/executor/helpers.go b/mdl/executor/helpers.go index 4e984d8c6a..b2a127eee8 100644 --- a/mdl/executor/helpers.go +++ b/mdl/executor/helpers.go @@ -288,9 +288,19 @@ func validateWidgetReferences(ctx *ExecContext, widgets []*ast.WidgetV3, sc *scr } if len(refs.images) > 0 { - // No same-script exemption: MDL cannot create an image collection entry, - // so an image reference can only ever resolve against the project. + // The project's images plus the ones this script creates + // (mendixlabs/mxcli#1419). A nil set means the project could not be read, + // and stays nil: imageRefErrors then reports nothing rather than calling + // every other collection missing. known, collections := buildImageQualifiedNames(ctx) + if known != nil && collections != nil { + for qn := range sc.imageCollections { + collections[qn] = true + } + for qn := range sc.images { + known[qn] = true + } + } errors = append(errors, imageRefErrors(refs.images, known, collections)...) } @@ -598,6 +608,24 @@ func buildQueueQualifiedNames(ctx *ExecContext) map[string]bool { return result } +// buildImportMappingQualifiedNames returns the set of import mapping qualified +// names in the project, matched exactly as GetImportMappingByQualifiedName does. +func buildImportMappingQualifiedNames(ctx *ExecContext) map[string]bool { + result := make(map[string]bool) + h, err := getHierarchy(ctx) + if err != nil { + return result + } + ims, err := ctx.Backend.ListImportMappings() + if err != nil { + return result + } + for _, im := range ims { + result[h.GetQualifiedName(im.ContainerID, im.Name)] = true + } + return result +} + // buildNanoflowQualifiedNames returns a set of all nanoflow qualified names in the project. func buildNanoflowQualifiedNames(ctx *ExecContext) map[string]bool { result := make(map[string]bool) diff --git a/mdl/executor/import_mapping_unresolved_pedapp_test.go b/mdl/executor/import_mapping_unresolved_pedapp_test.go new file mode 100644 index 0000000000..84b4ee02e6 --- /dev/null +++ b/mdl/executor/import_mapping_unresolved_pedapp_test.go @@ -0,0 +1,171 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "context" + "strings" + "testing" + + "go.mongodb.org/mongo-driver/bson" + + "github.com/mendixlabs/mxcli/model" +) + +const imm1418Prelude = `create json structure MyFirstModule.JSON_D sample '{"code": "NL", "name": "Netherlands"}'; +create persistent entity MyFirstModule.CountryD ( Code: string, Name: string ); +` + +const imm1418Microflow = `create or modify microflow MyFirstModule.MF_D ( $P: string ) +begin + $R = import from mapping MyFirstModule.IMM_D($P); +end; +` + +const imm1418Mapping = `create import mapping MyFirstModule.IMM_D with json structure MyFirstModule.JSON_D +{ create MyFirstModule.CountryD { Code = code, Name = name } }; +` + +// mfUnitByName returns a microflow's unit ID and stored bytes. +func mfUnitByName(t *testing.T, exec *Executor, module, name string) (model.ID, []byte) { + t.Helper() + ctx := exec.newExecContext(context.Background()) + h, err := getHierarchy(ctx) + if err != nil { + t.Fatal(err) + } + all, err := ctx.Backend.ListMicroflows() + if err != nil { + t.Fatal(err) + } + for _, m := range all { + if m.Name == name && h.GetModuleName(h.FindModuleID(m.ContainerID)) == module { + raw, err := ctx.Backend.GetRawUnitBytes(m.ID) + if err != nil { + t.Fatal(err) + } + return m.ID, append([]byte(nil), raw...) + } + } + return "", nil +} + +// entityTypeEntities collects the Entity of every DataTypes$ObjectType / +// ListType in a stored document, and when set != nil replaces it. +func entityTypeEntities(v any, set *string, out *[]string) any { + switch x := v.(type) { + case bson.D: + typ, _ := afGet(x, "$Type").(string) + isEntityType := typ == "DataTypes$ObjectType" || typ == "DataTypes$ListType" + for i := range x { + if isEntityType && x[i].Key == "Entity" { + s, _ := x[i].Value.(string) + *out = append(*out, s) + if set != nil { + x[i].Value = *set + } + continue + } + x[i].Value = entityTypeEntities(x[i].Value, set, out) + } + return x + case bson.A: + for i := range x { + x[i] = entityTypeEntities(x[i], set, out) + } + return x + } + return v +} + +// mendixlabs/mxcli#1418, at the layer the symptom lives in: a real project +// write. Exec of a microflow calling a mapping created further down the script +// used to "Create microflow" with VariableType.Entity = "", which mx check +// cannot load (ArgumentNullException in EntityType.set_EntityId). +func TestImportMappingForwardRefExecRefuses1418(t *testing.T) { + exec, _ := openPedAppFixture(t) + err := agreeExec(t, exec, "mdl 1;\n"+imm1418Prelude+imm1418Microflow+imm1418Mapping) + if err == nil || !strings.Contains(err.Error(), "import mapping not found") { + var got []string + if _, raw := mfUnitByName(t, exec, "MyFirstModule", "MF_D"); raw != nil { + var d bson.D + if uerr := bson.Unmarshal(raw, &d); uerr == nil { + entityTypeEntities(d, nil, &got) + } + } + t.Fatalf("exec err = %v, want the import mapping refused; stored result entities: %q", err, got) + } + if id, _ := mfUnitByName(t, exec, "MyFirstModule", "MF_D"); id != "" { + t.Fatalf("the refused microflow was written anyway (%s)", id) + } + + // Control: the same statements in definition order execute. + if err := agreeExec(t, exec, "mdl 1;\n"+imm1418Mapping+imm1418Microflow); err != nil { + t.Fatalf("control: mapping-first order failed: %v", err) + } + _, raw := mfUnitByName(t, exec, "MyFirstModule", "MF_D") + var d bson.D + if err := bson.Unmarshal(raw, &d); err != nil { + t.Fatal(err) + } + var got []string + entityTypeEntities(d, nil, &got) + if len(got) != 1 || got[0] != "MyFirstModule.CountryD" { + t.Fatalf("stored result entities = %q, want [MyFirstModule.CountryD]", got) + } +} + +// A project already written by the old bug: re-running the statement once the +// mapping exists printed "Unchanged microflow" and kept the empty entity, +// because describe prints the broken activity as the very statement that would +// repair it. mdl 0 now rebuilds it (healing the project); mdl 1 refuses with +// the reason instead of reporting Unchanged. +func TestImportMappingEmptyStoredEntityIsNotUnchanged1418(t *testing.T) { + exec, out := openPedAppFixture(t) + if err := agreeExec(t, exec, "mdl 1;\n"+imm1418Prelude+imm1418Mapping+imm1418Microflow); err != nil { + t.Fatal(err) + } + corrupt := func() { + t.Helper() + id, raw := mfUnitByName(t, exec, "MyFirstModule", "MF_D") + var d bson.D + if err := bson.Unmarshal(raw, &d); err != nil { + t.Fatal(err) + } + empty := "" + var got []string + entityTypeEntities(d, &empty, &got) + if len(got) != 1 { + t.Fatalf("expected one entity type in MF_D, found %q", got) + } + b, err := bson.Marshal(d) + if err != nil { + t.Fatal(err) + } + if err := exec.newExecContext(context.Background()).Backend.UpdateRawUnit(string(id), b); err != nil { + t.Fatal(err) + } + } + + corrupt() + out.Reset() + err := agreeExec(t, exec, "mdl 1;\n"+imm1418Microflow) + if err == nil || !strings.Contains(err.Error(), "no result entity") { + t.Fatalf("mdl 1 re-run over a stored empty entity: err = %v, output %q; want a refusal naming the missing result entity", err, out.String()) + } + + out.Reset() + if err := agreeExec(t, exec, imm1418Microflow); err != nil { + t.Fatalf("mdl 0 re-run: %v", err) + } + _, raw := mfUnitByName(t, exec, "MyFirstModule", "MF_D") + var d bson.D + if err := bson.Unmarshal(raw, &d); err != nil { + t.Fatal(err) + } + var got []string + entityTypeEntities(d, nil, &got) + if len(got) != 1 || got[0] != "MyFirstModule.CountryD" { + t.Fatalf("after the mdl 0 re-run the stored result entities are %q, want [MyFirstModule.CountryD] (output %q)", got, out.String()) + } +} diff --git a/mdl/executor/import_mapping_unresolved_test.go b/mdl/executor/import_mapping_unresolved_test.go new file mode 100644 index 0000000000..519327b7b8 --- /dev/null +++ b/mdl/executor/import_mapping_unresolved_test.go @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "errors" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// mendixlabs/mxcli#1418: `import from mapping X` written before `create import +// mapping X` stored the activity with VariableType.Entity = "", and Mendix +// cannot load the project ("ArgumentNullException ... EntityType.set_EntityId"). +// The lookup error was dropped and an empty result entity written. The builder +// must refuse the statement instead. +func TestImportFromMappingRefusesUnresolvedMapping(t *testing.T) { + tests := []struct { + name string + lookup func(moduleName, name string) (*model.ImportMapping, error) + want string + }{ + { + name: "mapping not found", + lookup: func(moduleName, name string) (*model.ImportMapping, error) { + return nil, errors.New("import mapping not found: " + moduleName + "." + name) + }, + want: "not found", + }, + { + name: "mapping has no root entity", + lookup: func(moduleName, name string) (*model.ImportMapping, error) { + return &model.ImportMapping{Elements: []*model.ImportMappingElement{{Kind: "Object"}}}, nil + }, + want: "no root entity", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fb := &flowBuilder{ + varTypes: map[string]string{}, + backend: &mock.MockBackend{GetImportMappingByQualifiedNameFunc: tt.lookup}, + } + fb.addImportFromMappingAction(&ast.ImportFromMappingStmt{ + OutputVariable: "R", + SourceVariable: "P", + Mapping: ast.QualifiedName{Module: "MyFirstModule", Name: "IMM_D"}, + }) + if len(fb.errors) == 0 { + var entity model.ID + for _, o := range fb.objects { + if act, ok := o.(*microflows.ActionActivity); ok { + if ix, ok := act.Action.(*microflows.ImportXmlAction); ok && ix.ResultHandling != nil { + entity = ix.ResultHandling.ResultEntityID + } + } + } + t.Fatalf("no builder error; activity written with ResultEntityID=%q (Mendix cannot load an empty entity)", entity) + } + msg := strings.Join(fb.errors, "\n") + if !strings.Contains(msg, "MyFirstModule.IMM_D") || !strings.Contains(msg, tt.want) { + t.Fatalf("error = %q, want it to name MyFirstModule.IMM_D and %q", msg, tt.want) + } + }) + } +} + +// MDL-ORDER01 must report the forward reference at check time, before exec +// writes the statements ahead of it. +func TestValidateScriptDefinitionOrder_ImportMappingForwardRef(t *testing.T) { + msgs := buildOrderProgram(t, `create json structure MyFirstModule.JSON_D sample '{"code": "NL"}'; +create persistent entity MyFirstModule.CountryD ( Code: string ); +create microflow MyFirstModule.MF_D ( $P: string ) +begin + $R = import from mapping MyFirstModule.IMM_D($P); +end; +create import mapping MyFirstModule.IMM_D with json structure MyFirstModule.JSON_D +{ create MyFirstModule.CountryD { Code = code } };`) + if len(msgs) != 1 || !strings.Contains(msgs[0], "import mapping MyFirstModule.IMM_D") { + t.Fatalf("violations = %q, want one naming import mapping MyFirstModule.IMM_D", msgs) + } + + // The backward order is fine. + msgs = buildOrderProgram(t, `create json structure MyFirstModule.JSON_D sample '{"code": "NL"}'; +create persistent entity MyFirstModule.CountryD ( Code: string ); +create import mapping MyFirstModule.IMM_D with json structure MyFirstModule.JSON_D +{ create MyFirstModule.CountryD { Code = code } }; +create microflow MyFirstModule.MF_D ( $P: string ) +begin + $R = import from mapping MyFirstModule.IMM_D($P); +end;`) + if len(msgs) != 0 { + t.Fatalf("backward reference flagged: %q", msgs) + } +} diff --git a/mdl/executor/issue707_remaining_test.go b/mdl/executor/issue707_remaining_test.go index e47b86e4e2..851fbc92ad 100644 --- a/mdl/executor/issue707_remaining_test.go +++ b/mdl/executor/issue707_remaining_test.go @@ -48,7 +48,10 @@ func userRoleCtx(t *testing.T, roles ...*security.UserRole) (*ExecContext, *byte IsConnectedFunc: func() bool { return true }, GetProjectSecurityFunc: func() (*security.ProjectSecurity, error) { return ps, nil }, AddUserRoleFunc: func(_ model.ID, name string, mrs []string, manageAll bool) error { - ps.UserRoles = append(ps.UserRoles, &security.UserRole{Name: name, ModuleRoles: mrs, ManageAllRoles: manageAll}) + // CheckSecurity: true is what the real writer gives a new role + // (Security$UserRole.CheckSecurity defaults to true), so a replay + // that does not state it gets true, as on disk (#1427). + ps.UserRoles = append(ps.UserRoles, &security.UserRole{Name: name, ModuleRoles: mrs, ManageAllRoles: manageAll, CheckSecurity: true}) return nil }, AlterUserRoleModuleRolesFunc: func(_ model.ID, name string, add bool, mrs []string) error { @@ -134,11 +137,38 @@ func TestIssue707_UserRoleWithoutModuleRolesReparses(t *testing.T) { assertNoError(t, describeUserRole(ctx, ast.QualifiedName{Name: "Empty"})) out := buf.String() stmt := findStmt[*ast.CreateUserRoleStmt](t, reparse(t, out), out) - if stmt.Name != "Empty" || len(stmt.ModuleRoles) != 0 || stmt.Description != nil || stmt.CheckSecurity != nil { + if stmt.Name != "Empty" || len(stmt.ModuleRoles) != 0 || stmt.Description != nil || + stmt.CheckSecurity == nil || *stmt.CheckSecurity { // stated even at false (#1427) t.Errorf("empty role re-parses as %+v\n%s", stmt, out) } } +// mendixlabs/mxcli#1427: describe printed `CheckSecurity` only when true, but a +// new role is written with true, so a role with security checking off came back +// from describe → drop → replay with it silently re-enabled. Replayed onto an +// existing role with it on, the unstated property kept true as well. +func TestIssue1427_UserRoleCheckSecurityFalseSurvivesReplay(t *testing.T) { + stored := &security.UserRole{Name: "UserA", ModuleRoles: []string{"ModA.RoleA"}, CheckSecurity: false} + src, srcBuf, _ := userRoleCtx(t, stored) + assertNoError(t, describeUserRole(src, ast.QualifiedName{Name: "UserA"})) + out := srcBuf.String() + stmt := findStmt[*ast.CreateUserRoleStmt](t, reparse(t, out), out) + + // Replay onto a project without the role (describe, drop, recreate). + dst, _, ps := userRoleCtx(t) + assertNoError(t, execCreateUserRole(dst, stmt)) + if len(ps.UserRoles) != 1 || ps.UserRoles[0].CheckSecurity { + t.Errorf("recreated role has CheckSecurity %v, want false\n%s", ps.UserRoles[0].CheckSecurity, out) + } + + // Replay onto an existing role that has it on. + mod, _, mps := userRoleCtx(t, &security.UserRole{Name: "UserA", ModuleRoles: []string{"ModA.RoleA"}, CheckSecurity: true}) + assertNoError(t, execCreateUserRole(mod, stmt)) + if mps.UserRoles[0].CheckSecurity { + t.Errorf("modified role kept CheckSecurity true, want false\n%s", out) + } +} + // Control: `create or modify` states a property only when the statement does, // so a replay of the old positional form onto an existing role keeps the // stored description rather than clearing it. diff --git a/mdl/executor/output_guard.go b/mdl/executor/output_guard.go index f0ae989897..f9dc5bd174 100644 --- a/mdl/executor/output_guard.go +++ b/mdl/executor/output_guard.go @@ -48,8 +48,27 @@ func (g *outputGuard) Write(p []byte) (int, error) { g.exceeded = true // Write the current chunk first so output isn't abruptly cut mid-line. _, _ = g.w.Write(p) - return len(p), mdlerrors.NewValidationf("output line limit exceeded (%d lines); statement aborted", g.maxLines) + return len(p), g.limitError() } return g.w.Write(p) } + +// exceededError returns the limit error if the current statement went past the +// line limit, or nil. Execute checks it after the statement returns: most +// handlers (every describer) ignore Fprintf's error, so the one Write returned +// never reaches the caller, and without this check the statement succeeded +// with its output silently cut off (mendixlabs/mxcli#1425). +func (g *outputGuard) exceededError() error { + g.mu.Lock() + defer g.mu.Unlock() + if !g.exceeded { + return nil + } + return g.limitError() +} + +func (g *outputGuard) limitError() error { + return mdlerrors.NewValidationf("output line limit exceeded (%d lines); output truncated and statement aborted. "+ + "Describe a large module per document (e.g. describe entity / microflow / page ...) instead of all at once", g.maxLines) +} diff --git a/mdl/executor/output_guard_test.go b/mdl/executor/output_guard_test.go new file mode 100644 index 0000000000..ec040e9213 --- /dev/null +++ b/mdl/executor/output_guard_test.go @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "fmt" + "reflect" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// A statement whose output passes the per-statement line limit must fail, not +// exit 0 with the text cut off mid-statement. Describers ignore Fprintf's +// error, so the guard's own error never reached the caller and +// `describe module … with all` stopped at exactly 10,000 lines with no +// warning (mendixlabs/mxcli#1425). The statement under the limit is the +// control. +func TestExecuteReportsOutputLineLimitExceeded(t *testing.T) { + for _, tc := range []struct { + name string + lines int + wantErr bool + }{ + {"under the limit", maxOutputLines, false}, + {"over the limit", maxOutputLines + 500, true}, + } { + t.Run(tc.name, func(t *testing.T) { + var out bytes.Buffer + e := New(&out) + e.registry.handlers[reflect.TypeOf(&ast.DescribeStmt{})] = func(ctx *ExecContext, _ ast.Statement) error { + for i := 0; i < tc.lines; i++ { + fmt.Fprintf(ctx.Output, "line %d\n", i) // error ignored, as describers do + } + return nil + } + err := e.Execute(&ast.DescribeStmt{}) + if !tc.wantErr { + if err != nil { + t.Fatalf("Execute: %v", err) + } + return + } + if err == nil { + t.Fatalf("Execute returned nil after %d lines; output silently truncated at %d", tc.lines, strings.Count(out.String(), "\n")) + } + if !strings.Contains(err.Error(), "truncated") { + t.Fatalf("error does not say the output was truncated: %v", err) + } + }) + } +} diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index 5f7ba79233..b575d56e5b 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -51,6 +51,14 @@ type scriptContext struct { // (mendixlabs/mxcli#1211). queues map[string]bool + // Image collections created (Module.Collection) and the images they declare + // (Module.Collection.Image), both lower-cased like buildImageQualifiedNames. + // Missing, an image widget naming a collection the same script creates was + // "image collection not found" and exec refused the script + // (mendixlabs/mxcli#1419). + imageCollections map[string]bool + images map[string]bool + // Microflow-typed parameters of the Java actions the script creates // (Module.Action -> parameter name). Such a parameter takes a microflow // NAME, never an expression (mendixlabs/mxcli#1210). @@ -133,6 +141,8 @@ func newScriptContext() *scriptContext { menus: make(map[string]bool), constants: make(map[string]bool), queues: make(map[string]bool), + imageCollections: make(map[string]bool), + images: make(map[string]bool), javaActionFlowParams: make(map[string]map[string]bool), entityDecls: make(map[string]*ast.CreateEntityStmt), @@ -292,6 +302,14 @@ func (sc *scriptContext) collectSingle(stmt ast.Statement) { if s.Name.Module != "" { sc.menus[s.Name.String()] = true } + case *ast.CreateImageCollectionStmt: + if s.Name.Module != "" { + qn := strings.ToLower(s.Name.String()) + sc.imageCollections[qn] = true + for _, img := range s.Images { + sc.images[qn+"."+strings.ToLower(img.Name)] = true + } + } case *ast.CreateWorkflowStmt: if s.Name.Module != "" { sc.workflows[s.Name.String()] = true @@ -711,7 +729,7 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext // Reported together with the reference errors below rather than // instead of them: a body error used to hide a call's unknown // parameter, which mxbuild reports as well (CE1613, #953). - validationErrors := validateFlowBody(s.Parameters, s.Body, true, checkAssociationShapes(ctx, sc)) + validationErrors := validateFlowBody(s.Parameters, s.ReturnType, s.Body, true, checkAssociationShapes(ctx, sc)) // Validate references inside microflow body (pages, microflows, java actions, entities) refErrors := validateFlowBodyReferences(ctx, s.Body, sc) if ctx.Connected() { @@ -764,7 +782,7 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext // Reported together with the reference errors below rather than // instead of them: a body error used to hide a call's unknown // parameter, which mxbuild reports as well (CE1613, #953). - validationErrors := validateFlowBody(s.Parameters, s.Body, true, checkAssociationShapes(ctx, sc)) + validationErrors := validateFlowBody(s.Parameters, s.ReturnType, s.Body, true, checkAssociationShapes(ctx, sc)) // Validate references inside nanoflow body (an excluded nanoflow's are warnings) refErrors := validateFlowBodyReferences(ctx, s.Body, sc) if ctx.Connected() { diff --git a/mdl/executor/validate_definition_order.go b/mdl/executor/validate_definition_order.go index 395afa6e9d..0fec65d31f 100644 --- a/mdl/executor/validate_definition_order.go +++ b/mdl/executor/validate_definition_order.go @@ -38,6 +38,10 @@ const ( defNanoflow = "nanoflow" defPage = "page" defQueue = "task queue" + // defImportMapping is the target of `import from mapping`, which the flow + // builder resolves when it writes the activity — it needs the mapping's + // root entity for the result type (mendixlabs/mxcli#1418). + defImportMapping = "import mapping" ) // defKey identifies a document by kind and qualified name. Kind is part of the @@ -160,6 +164,8 @@ func definedBy(stmt ast.Statement) (key defKey, plain bool) { return qualifiedKey(defPage, s.Name), !s.IsModify && !s.IsReplace case *ast.CreateQueueStmt: return qualifiedKey(defQueue, s.Name), !s.CreateOrModify + case *ast.CreateImportMappingStmt: + return qualifiedKey(defImportMapping, s.Name), !s.CreateOrModify } return defKey{}, false } @@ -271,6 +277,10 @@ func flowDefRefs(site string, params []ast.MicroflowParam, ret *ast.MicroflowRet // The action itself resolves lazily (measured, #955); its IN QUEUE // does not — buildQueueSettings refuses a queue it cannot find. out = append(out, queueDefRef(c.Queue, site)...) + case *ast.ImportFromMappingStmt: + // Resolved for its root entity, which the activity stores as its + // result type; exec refuses one it cannot find (#1418). + out = append(out, oneDefRef(c.Mapping, defImportMapping, site+": import from mapping")...) } } return out @@ -424,6 +434,8 @@ func validateForwardDefRefs(ctx *ExecContext, prog *ast.Program) []error { m = buildPageQualifiedNames(ctx) case defQueue: m = buildQueueQualifiedNames(ctx) + case defImportMapping: + m = buildImportMappingQualifiedNames(ctx) } stored[kind] = m return m @@ -432,8 +444,9 @@ func validateForwardDefRefs(ctx *ExecContext, prog *ast.Program) []error { m := storedOf(kind) // An empty listing is a backend that could not answer, as often as a // project without any — silence, not a finding (cf. checkAlterTarget). - // Queues are the exception: a project with none is ordinary. - if len(m) == 0 && kind != defQueue { + // Queues and import mappings are the exception: a project with none + // is ordinary. + if len(m) == 0 && kind != defQueue && kind != defImportMapping { return true } if kind == defQueue { diff --git a/mdl/executor/validate_merge_scope_test.go b/mdl/executor/validate_merge_scope_test.go index 46a1a870f1..c789e90aa6 100644 --- a/mdl/executor/validate_merge_scope_test.go +++ b/mdl/executor/validate_merge_scope_test.go @@ -45,6 +45,33 @@ func createMicroflowStmtOf(t *testing.T, src string) *ast.CreateMicroflowStmt { return nil } +// mendixlabs/mxcli#1436: `returns Boolean as $Closed` names the result for +// callers and declares nothing, so `set $Closed = true` is correctly refused — +// but "variable 'Closed' is not declared" read as contradicting the very +// clause that names it. The message must say what `as $Closed` does. +func TestValidateFlowBody_SetOnReturnsAsNameExplainsTheClause(t *testing.T) { + stmt := createMicroflowStmtOf(t, "create microflow M.F () returns Boolean as $Closed\nbegin\n set $Closed = true;\n return $Closed;\nend;") + errs := ValidateMicroflowBody(stmt) + if len(errs) != 1 { + t.Fatalf("want exactly one error (semantics unchanged), got %v", errs) + } + for _, want := range []string{"is not declared", "returns … as $Closed", "declare $Closed Boolean"} { + if !strings.Contains(errs[0], want) { + t.Errorf("error should contain %q, got: %s", want, errs[0]) + } + } +} + +// Control: another undeclared variable keeps the generic message, with no +// mention of the `as` clause. +func TestValidateFlowBody_SetOnOtherUndeclaredNameKeepsGenericMessage(t *testing.T) { + stmt := createMicroflowStmtOf(t, "create microflow M.F () returns Boolean as $Closed\nbegin\n set $Other = true;\n return true;\nend;") + errs := ValidateMicroflowBody(stmt) + if len(errs) != 1 || strings.Contains(errs[0], "returns …") { + t.Fatalf("want the generic not-declared error, got %v", errs) + } +} + func TestValidateFlowBody_MergeSeesItsJoinsVariables(t *testing.T) { stmt := createMicroflowStmtOf(t, mergeSectionScopeMDL) if errs := ValidateMicroflowBody(stmt); len(errs) > 0 { diff --git a/mdl/executor/validate_widget_image_ref_script_test.go b/mdl/executor/validate_widget_image_ref_script_test.go new file mode 100644 index 0000000000..0fa42568da --- /dev/null +++ b/mdl/executor/validate_widget_image_ref_script_test.go @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// mendixlabs/mxcli#1419: an image collection created earlier in the same script +// was invisible to the reference pre-check, so +// +// create image collection ModA.Icons (image pix from file '…'); +// create page … { image img1 (Image: 'ModA.Icons.pix') } +// +// was refused by check with +// +// image collection not found: ModA.Icons (referenced as ModA.Icons.pix) +// +// and by exec with "Refusing to execute: 1 unresolved reference(s) above". +// Split into two scripts, the same statements passed. + +func scriptWithImageCollection(module, name string, images ...string) *scriptContext { + stmt := &ast.CreateImageCollectionStmt{Name: ast.QualifiedName{Module: module, Name: name}} + for _, img := range images { + stmt.Images = append(stmt.Images, ast.ImageItem{Name: img}) + } + sc := newScriptContext() + sc.collectSingle(stmt) + return sc +} + +func TestWidgetRefs_ImageInScriptCreatedCollectionIsResolved(t *testing.T) { + ctx := imageCollectionCtx(t) + sc := scriptWithImageCollection("ModA", "Icons", "pix") + + errs := validateWidgetReferences(ctx, imageRefWidget("image", "Image", "ModA.Icons.pix"), sc) + if len(errs) != 0 { + t.Fatalf("an image in a collection the script creates was refused: %v", errs) + } + // Mendix resolves names case-insensitively; the script's spelling must too. + errs = validateWidgetReferences(ctx, imageRefWidget("staticimage", "Image", "moda.ICONS.Pix"), sc) + if len(errs) != 0 { + t.Fatalf("a case difference against a script-created image was refused: %v", errs) + } + // The project's own collections still resolve alongside the script's. + errs = validateWidgetReferences(ctx, imageRefWidget("image", "Image", "MyFirstModule.Images.gallery"), sc) + if len(errs) != 0 { + t.Fatalf("a project image was refused once the script defined a collection: %v", errs) + } +} + +// CONTROL: the script's collection is a real answer, not an exemption — an +// image it does not declare is still reported. +func TestWidgetRefs_MissingImageInScriptCreatedCollectionIsReported(t *testing.T) { + ctx := imageCollectionCtx(t) + sc := scriptWithImageCollection("ModA", "Icons", "pix") + + errs := validateWidgetReferences(ctx, imageRefWidget("image", "Image", "ModA.Icons.nope"), sc) + if len(errs) != 1 || !strings.Contains(errs[0], "image not found: nope") { + t.Fatalf("an image the script's collection does not hold was accepted; got %v", errs) + } +} diff --git a/mdl/roundtrip/allowlist_test.go b/mdl/roundtrip/allowlist_test.go index 728126edbf..f83d25a037 100644 --- a/mdl/roundtrip/allowlist_test.go +++ b/mdl/roundtrip/allowlist_test.go @@ -30,70 +30,56 @@ var knownFailures = map[string]knownFailure{ // Plain `create` where `create or modify` is needed (#705 item 5). // #721 G: describe output that does not parse. - "building block Atlas_Web_Content.Alert": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.AlertIcon": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.AlertIcon_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Alert_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Breadcrumb": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Breadcrumb_Underline": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Card": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Card_Action": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Card_ActionWithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Card_Background": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Card_WithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Form_Horizontal": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Form_Horizontal_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Form_Horizontal_WithTitle": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Form_Vertical": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Form_Vertical_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Form_Vertical_WithTitle": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Heroheader": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Heroheader_Background": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Heroheader_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.ListItem_DoubleLine": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.ListItem_SingleLine": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.ListItem_WithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.List_Cards": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.List_WithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Master_Detail": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Pageheader": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.PageheaderImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.PageheaderImage_WithBack": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.PageheaderImage_WithControls": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Pageheader_WithBack": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Pageheader_WithControls": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Pageheader_WithSearch": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Timeline": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Timeline_WithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Wizard_Arrow": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Wizard_Arrow_Step": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Wizard_Circle": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Wizard_Circle_Step": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block FeedbackModule.FeedbackWidget": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "javascript action DataWidgets.Reset_All_Filters": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action DataWidgets.Reset_Filter": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action DataWidgets.Set_Filter": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action FeedbackModule.JS_GetFeedbackStorageObject": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.DraftEmail": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.FindObjectWithGUID": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, - "javascript action NanoflowCommons.Geocode": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.GetCurrentLocation": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.GetCurrentLocationMinimumAccuracy": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.GetGuid": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, - "javascript action NanoflowCommons.GetObjectByGuid": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, - "javascript action NanoflowCommons.GetStorageItemObject": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, - "javascript action NanoflowCommons.GetStorageItemObjectList": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, - "javascript action NanoflowCommons.RefreshEntity": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.RefreshObject": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, - "javascript action NanoflowCommons.ReverseGeocode": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.SetStorageItemObject": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, - "javascript action NanoflowCommons.SetStorageItemObjectList": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, - "javascript action NanoflowCommons.SetStorageItemString": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.Share": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.ShowConfirmation": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.ShowProgress": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.SignIn": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action WebActions.TakePicture": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, + "building block Atlas_Web_Content.Alert": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.AlertIcon": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.AlertIcon_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Alert_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Breadcrumb": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Breadcrumb_Underline": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Card": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Card_Action": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Card_ActionWithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Card_Background": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Card_WithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Form_Horizontal": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Form_Horizontal_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Form_Horizontal_WithTitle": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Form_Vertical": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Form_Vertical_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Form_Vertical_WithTitle": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Heroheader": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Heroheader_Background": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Heroheader_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.ListItem_DoubleLine": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.ListItem_SingleLine": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.ListItem_WithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.List_Cards": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.List_WithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Master_Detail": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Pageheader": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.PageheaderImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.PageheaderImage_WithBack": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.PageheaderImage_WithControls": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Pageheader_WithBack": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Pageheader_WithControls": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Pageheader_WithSearch": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Timeline": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Timeline_WithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Wizard_Arrow": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Wizard_Arrow_Step": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Wizard_Circle": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Wizard_Circle_Step": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block FeedbackModule.FeedbackWidget": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "javascript action FeedbackModule.JS_GetFeedbackStorageObject": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, + "javascript action NanoflowCommons.FindObjectWithGUID": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, + "javascript action NanoflowCommons.GetGuid": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, + "javascript action NanoflowCommons.GetObjectByGuid": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, + "javascript action NanoflowCommons.GetStorageItemObject": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, + "javascript action NanoflowCommons.GetStorageItemObjectList": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, + "javascript action NanoflowCommons.RefreshEntity": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, + "javascript action NanoflowCommons.RefreshObject": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, + "javascript action NanoflowCommons.SetStorageItemObject": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, + "javascript action NanoflowCommons.SetStorageItemObjectList": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, // #721 E: menus — struck by ako/mxcli#980 (the menu item writer's missing // AlternativeText and Items). diff --git a/mdl/roundtrip/testapp_allowlist_test.go b/mdl/roundtrip/testapp_allowlist_test.go index cccdf57d39..0e0f2db999 100644 --- a/mdl/roundtrip/testapp_allowlist_test.go +++ b/mdl/roundtrip/testapp_allowlist_test.go @@ -122,30 +122,16 @@ var testAppKnownFailures = map[string]knownFailure{ "export mapping Services.OrdersExportMapping": {laws: []law{lawGetPut}, issue: "#721", why: "export mapping: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "import mapping Clients.SoapCustomerOrdersImportMapping": {laws: []law{lawGetPut}, issue: "#721", why: "import mapping: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "import mapping Clients.SoapOrdersImportMapping": {laws: []law{lawGetPut}, issue: "#721", why: "import mapping: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action DataWidgets.Reset_All_Filters": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action DataWidgets.Reset_Filter": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action DataWidgets.Set_Filter": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action FeedbackModule.JS_GetFeedbackStorageObject": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.DraftEmail": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action NanoflowCommons.FindObjectWithGUID": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.Geocode": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.GetCurrentLocation": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.GetCurrentLocationMinimumAccuracy": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action NanoflowCommons.GetGuid": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action NanoflowCommons.GetObjectByGuid": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action NanoflowCommons.GetStorageItemObject": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action NanoflowCommons.GetStorageItemObjectList": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action NanoflowCommons.RefreshEntity": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action NanoflowCommons.RefreshObject": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.ReverseGeocode": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action NanoflowCommons.SetStorageItemObject": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action NanoflowCommons.SetStorageItemObjectList": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.SetStorageItemString": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.Share": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.ShowConfirmation": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.ShowProgress": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.SignIn": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action WebActions.TakePicture": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "page Administration.Account_Overview": {laws: []law{lawGetPut}, issue: "#721", why: "page: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "page FeedbackModule.PopupFailure": {laws: []law{lawGetPut}, issue: "#721", why: "page: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "page FeedbackModule.PopupSuccess": {laws: []law{lawGetPut}, issue: "#721", why: "page: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, diff --git a/mdl/visitor/visitor_list_activities.go b/mdl/visitor/visitor_list_activities.go index 139ec1c3f9..98189e43f5 100644 --- a/mdl/visitor/visitor_list_activities.go +++ b/mdl/visitor/visitor_list_activities.go @@ -354,9 +354,10 @@ func (b *Builder) ExitSetStatement(ctx *parser.SetStatementContext) { } if ctx.SET() == nil { if b.gate(setIsMandatory, ctx) { - b.addError(fmt.Errorf("line %d: a reassignment says `set` under mdl 1: write `set %s = %s;`. "+ + b.addError(fmt.Errorf("line %d: a reassignment says `set` under mdl 1: write `set %s = %s;`, "+ + "or `declare %s = %s;` if %s is a new variable. "+ "A List operation or Aggregate list activity is written without it, as its own statement "+ - "(`%s = filter $List where …;`)", line, target, valueText, target)) + "(`%s = filter $List where …;`)", line, target, valueText, target, valueText, target, target)) return } b.fixLastNote(setIsMandatory.Code, &ast.Fix{Edits: []ast.TextEdit{insertAt(ctx.GetStart().GetStart(), "set ")}}, "") diff --git a/mdl/visitor/visitor_list_activity_test.go b/mdl/visitor/visitor_list_activity_test.go index 2d1ade9f93..98ea03f895 100644 --- a/mdl/visitor/visitor_list_activity_test.go +++ b/mdl/visitor/visitor_list_activity_test.go @@ -320,6 +320,24 @@ func TestNestedListOperationRefusedUnderMdl1(t *testing.T) { } } +// mendixlabs/mxcli#1436: `$Closed = true;` on a name that was never declared +// (here: one named only by `returns Boolean as $Closed`) was told to write +// `set`, and `set` then failed with "not declared". The message must also +// offer the declaration for a variable that is new. +func TestSetIsMandatory_MessageOffersDeclareForANewVariable(t *testing.T) { + src := "mdl 1;\ncreate or modify microflow M.F () returns Boolean as $Closed\nbegin\n $Closed = true;\n return $Closed;\nend;" + _, errs := Build(src) + if len(errs) == 0 { + t.Fatal("mdl 1 must refuse a reassignment without set") + } + msg := errs[0].Error() + for _, want := range []string{"set $Closed = true;", "declare $Closed = true;"} { + if !strings.Contains(msg, want) { + t.Errorf("error should contain %q, got: %s", want, msg) + } + } +} + // `set` is mandatory for reassignment under mdl 1. func TestSetIsMandatoryUnderMdl1(t *testing.T) { body := "declare $I Integer = 0;\n $I = 5;" diff --git a/mdl/visitor/visitor_odata.go b/mdl/visitor/visitor_odata.go index 65df93561a..afe50ffc9d 100644 --- a/mdl/visitor/visitor_odata.go +++ b/mdl/visitor/visitor_odata.go @@ -470,7 +470,7 @@ func parsePublishMicroflowBlock(ctx parser.IPublishMicroflowBlockContext) *ast.P if member.IdentifierOrKeyword() == nil { continue } - p := &ast.PublishedParamDef{Name: member.IdentifierOrKeyword().GetText()} + p := &ast.PublishedParamDef{Name: identifierOrKeywordText(member.IdentifierOrKeyword())} if sl := member.STRING_LITERAL(); sl != nil { p.ExposedName = unquoteStringLit(sl) } @@ -511,7 +511,7 @@ func parseExposeMembers(ctx parser.IExposeClauseContext) []*ast.PublishedMemberD } m := &ast.PublishedMemberDef{ - Name: member.IdentifierOrKeyword().GetText(), + Name: identifierOrKeywordText(member.IdentifierOrKeyword()), } // Optional AS 'ExposedName' diff --git a/mdl/visitor/visitor_odata_quoted_member_test.go b/mdl/visitor/visitor_odata_quoted_member_test.go new file mode 100644 index 0000000000..f73c4c977c --- /dev/null +++ b/mdl/visitor/visitor_odata_quoted_member_test.go @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import "testing" + +// mendixlabs/mxcli#1423: `expose (..., "Description")` stored the double quotes +// as part of the member name, so mx check reported +// CE1613 "The selected attribute 'ModA.EntA."Description"' no longer exists". +// The quotes are identifier-quoting syntax, as in `create persistent entity`. +func TestPublishEntity_QuotedExposeMemberIsUnquoted(t *testing.T) { + svc := odataActionService(t, + " publish entity T.EntA as 'EntAs' (ReadMode: source)\n"+ + " expose (Name as 'Name' (KEY), \"Description\");") + if len(svc.Entities) != 1 || len(svc.Entities[0].Members) != 2 { + t.Fatalf("expected 1 entity with 2 members, got %+v", svc.Entities) + } + if got := svc.Entities[0].Members[1].Name; got != "Description" { + t.Errorf("member name = %q, want %q", got, "Description") + } +} + +// Same grammar rule, published microflow parameters. +func TestPublishMicroflow_QuotedExposeParamIsUnquoted(t *testing.T) { + svc := odataActionService(t, + " publish microflow T.DoThing expose ( \"Description\" as 'description' );") + if len(svc.Microflows) != 1 || len(svc.Microflows[0].Parameters) != 1 { + t.Fatalf("expected 1 microflow with 1 parameter, got %+v", svc.Microflows) + } + if got := svc.Microflows[0].Parameters[0].Name; got != "Description" { + t.Errorf("parameter name = %q, want %q", got, "Description") + } +} diff --git a/modelsdk/codec/decoder.go b/modelsdk/codec/decoder.go index 8e7bb3f692..7b49f2bbaf 100644 --- a/modelsdk/codec/decoder.go +++ b/modelsdk/codec/decoder.go @@ -99,6 +99,9 @@ var fieldAliases = map[string]string{ "Type": "NewType", "NewType": "Type", "CaseValue": "NewCaseValue", + // gen now binds the case under its storage name (mendixlabs/mxcli#1410); + // a document holding the SDK name still reads. + "NewCaseValue": "CaseValue", } // DecodeChild decodes a single embedded document child from raw BSON by key. diff --git a/modelsdk/codec/encoder.go b/modelsdk/codec/encoder.go index 836091c242..3154b5567e 100644 --- a/modelsdk/codec/encoder.go +++ b/modelsdk/codec/encoder.go @@ -14,7 +14,9 @@ import ( // Encoder serializes Element trees back to BSON bytes. type Encoder struct { - // OmitKeys suppresses BSON keys, per $Type, for this encoder only. + // OmitKeys suppresses BSON keys, per $Type, for this encoder only — a key a + // gen property would emit and a key a registered Studio Pro default would add + // alike (a Microflow's StableId is the latter: mendixlabs/mxcli#1410). // // It exists for **version-floored properties**. Mendix introduces properties // over time, and a key introduced after the project's own version is one that @@ -210,27 +212,27 @@ func (e *Encoder) buildDoc(elem element.Element) (bson.D, error) { } } for _, name := range d.NullFields { - if !emitted[name] { + if !emitted[name] && !e.omits(elem.TypeName(), name) { doc = append(doc, bson.E{Key: name, Value: nil}) } } for _, name := range d.EmptyStringFields { - if !emitted[name] { + if !emitted[name] && !e.omits(elem.TypeName(), name) { doc = append(doc, bson.E{Key: name, Value: ""}) } } for _, name := range d.FalseFields { - if !emitted[name] { + if !emitted[name] && !e.omits(elem.TypeName(), name) { doc = append(doc, bson.E{Key: name, Value: false}) } } for _, name := range d.ZeroGUIDFields { - if !emitted[name] { + if !emitted[name] && !e.omits(elem.TypeName(), name) { doc = append(doc, bson.E{Key: name, Value: zeroGUIDBinary()}) } } for _, name := range d.FreshGUIDFields { - if !emitted[name] { + if !emitted[name] && !e.omits(elem.TypeName(), name) { doc = append(doc, bson.E{Key: name, Value: idToBinarySubtype0("")}) } } diff --git a/modelsdk/gen/microflows/types.go b/modelsdk/gen/microflows/types.go index 6da0160443..1f89209a18 100644 --- a/modelsdk/gen/microflows/types.go +++ b/modelsdk/gen/microflows/types.go @@ -10619,7 +10619,8 @@ func (o *SequenceFlow) InitFromRaw(raw bson.Raw) { if child, err := codec.DecodeChild(raw, "Line"); err == nil { o.line.SetFromDecode(child) } - if child, err := codec.DecodeChild(raw, "CaseValue"); err == nil { + // STORAGE-NAME OVERRIDE, decode half — see initSequenceFlow. + if child, err := codec.DecodeChild(raw, "NewCaseValue"); err == nil { o.caseValue.SetFromDecode(child) } if children, err := codec.DecodeChildren(raw, "CaseValues"); err == nil { @@ -16217,7 +16218,12 @@ func initSequenceFlow() *SequenceFlow { o.lineType.Bind(&o.Base, 6) o.line = property.NewPart[element.Element]("Line") o.line.Bind(&o.Base, 7) - o.caseValue = property.NewPart[element.Element]("CaseValue") + // STORAGE-NAME OVERRIDE: the single case (Model SDK caseValue, deleted in + // 10.15.0 for the caseValues list) is stored under "NewCaseValue" — measured + // on every sequence flow of a Studio Pro 9.24.30 document (mendixlabs/mxcli#1410). + // Bound as "CaseValue" it could be read only through the decoder's alias and + // written under a key no Mendix version declares. + o.caseValue = property.NewPart[element.Element]("NewCaseValue") o.caseValue.Bind(&o.Base, 8) o.caseValues = property.NewPartList[element.Element]("CaseValues") o.caseValues.Bind(&o.Base, 9)