diff --git a/.claude/commands/mendix/lint.md b/.claude/commands/mendix/lint.md index 0b09c790c4..e72602c378 100644 --- a/.claude/commands/mendix/lint.md +++ b/.claude/commands/mendix/lint.md @@ -44,7 +44,7 @@ mxcli lint -p app.mpr --exclude System --exclude Administration | SEC001 | security | NoEntityAccessRules - Persistent entities need access rules | | SEC002 | security | WeakPasswordPolicy - Password minimum length should be 8+ | | SEC003 | security | DemoUsersActive - Demo users should be off at Production security | -| CONV011 | performance | NoCommitInLoop - Commit actions inside loops cause N+1 issues | +| CONV011 | performance | NoCommitInLoop - Commits inside loops (commit actions, or create/change with commit) cause N+1 issues | | CONV012 | quality | ExclusiveSplitCaption - Exclusive splits need meaningful captions | | CONV013 | quality | ErrorHandlingOnCalls - External calls (REST/WS/Java) need custom error handling | | CONV014 | quality | NoContinueErrorHandling - Don't silently swallow errors with Continue | diff --git a/.claude/skills/fix-issue/findings/mdl-backend.jsonl b/.claude/skills/fix-issue/findings/mdl-backend.jsonl index 69631cc904..fe0db9ae99 100644 --- a/.claude/skills/fix-issue/findings/mdl-backend.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-backend.jsonl @@ -172,3 +172,4 @@ {"area": "mdl/backend", "date": "2026-10-06", "symptom": "`describe microflow` prints `-- Unsupported action: Microflows$SendEmailAction` for the Studio Pro 11.13+ Send Email activity, and MDL has no statement to create it", "cause": "No reader case, and no grammar/AST/builder/writer. The vendored gen type `genMf.SendEmailAction`/`EmailMessage` predates 11.12: it binds Subject/MessageBody* as expressions, but 11.13 deleted those and stores SubjectTemplate/MessageBodyPlainTextTemplate/MessageBodyHtmlTemplate (Microflows$StringTemplate) plus a CustomHeaders list the gen type has no field for", "file": "mdl/backend/modelsdk/microflow_send_email.go", "insight": "The gen type existing (the issue cited it as 'the storage side seems known') was a false lead: it is the pre-11.12 shape, so both directions are raw-keyed against a Studio Pro-saved document (ako/TestApp Email.EmailMF) and a key-for-key shape test (TestSendEmailActionToGen_MatchesStudioProShape) pins types and markers — it caught ConnectionTimeout being int64 and the empty CustomHeaders list needing marker 3. Version facts come from mendixmodelsdk 4.116 StructureVersionInfo (templates 11.12.0, plain Subject deleted 11.13.0), hence the 11.13.0 gate. Run negative controls through mxbuild before writing check rules: of the plausible rules, 'check server identity needs SSL' and the header-name rule are ACCEPTED by mxbuild 11.15.0-rc.4 (so warnings), while type faults are CE9528 for address/host/port/user (not the CE0117 exprcheck's E009 used to name — slots can now carry their own Mxbuild code). Attachment is a bare variable name (CE0109 on a wrong one). Shapes MDL cannot restate (auth document, pre-11.13 expression subject, non-SMTP) stay UnsupportedAction. Studio Pro's Test Email tab (TestEmailMessage) is written empty on rewrite, by design. An unedited describe->exec round trip is elided as Unchanged, so it cannot demonstrate loss; author-from-script is the meaningful control (old binary: `mismatched input 'email' expecting REST`).", "refs": ["mendixlabs/mxcli#1315"], "ce": ["CE9528", "CE0117", "CE0720", "CE0109"], "rules": ["MDL-EMAIL01", "MDL-EMAIL02", "MDL-EMAIL03", "E009"]} {"area": "mdl/backend", "date": "2026-10-07", "symptom": "After `RENAME JAVA ACTION M.JA_Old TO JA_New` (or `mxcli rename java-action`), javasource/m/actions/JA_New.java still declares `public class JA_Old`, its constructor `public JA_Old(` and toString `return \"JA_Old\";` \u2014 javac: `class JA_Old is public, should be declared in a file named JA_Old.java`. `mxcli docker build` reports BUILD SUCCEEDED regardless", "cause": "Backend.RenameJavaSourceFile (mdl/backend/modelsdk/java_write.go) only os.Rename'd the file; nothing rewrote the generated parts that carry the action's name", "file": "mdl/backend/modelsdk/java_write.go, sdk/javaactions/rename.go", "insight": "A green mxbuild does NOT prove the javasource on disk compiles: mxbuild regenerates action stubs in the copy it builds (class, constructor, toString follow the model; user code, extra code and the import list are kept byte-for-byte, a mention of the old name inside them included), so the defect only shows where the on-disk file is compiled directly \u2014 `run --local --watch` hot reload, IDEs. The cheap probe is javac on the one file with runtime/bundles/com.mendix.public-api.jar on the classpath. The oracle for the rewrite is mxbuild itself: run the mxbuild binary on the project IN PLACE (not `docker build`, which uses a temp copy) and diff the javasource file \u2014 it changed exactly three lines; that pair is now testdata/mxbuild/JA_Renamed*. Rewrite those three spots, not a full GenerateSource, which would reformat a Studio Pro-authored file. Control: unfixed binary \u2192 javac error above; fixed \u2192 javac exit 0, docker build BUILD SUCCEEDED", "refs": ["follow-up to mendixlabs/mxcli#1318"]} {"area": "mdl/backend", "date": "2026-10-06", "symptom": "Opening a change in Studio Pro's Changes panel throws `System.InvalidOperationException: Objects with ID … of type Forms$MicroflowParameterMapping do not have the same properties. baseNames = Expression, Parameter, Variable; newNames = Parameter, Expression` after `alter page … set ('onClickAction': call microflow M.F(P = $P)) on w`. `mx check` is 0 errors; the page opens. Second, silent half: `$P` (a page parameter) is stored as the Expression \"$P\", which Studio Pro does not bind (CE1571, the #1140 form)", "cause": "(1) `bindParameterMappingValue` sets only Expression for an Expression-bound argument; the gen `Variable` Part stays unset and the encoder omits an unset Part on a new element, so the key vanished. Studio Pro writes all three keys, nulling the unused slot, and its merge library compares key sets per $ID — the canon $ID carry makes the new mapping the SAME object as the stored one, so the diff sees two shapes of one element. (2) `convertASTAction` (ALTER PAGE SET Action and named action slots) built its pageBuilder with no paramScope/localVariables, so `classifyFlowArgValue` could not recognise a page/snippet parameter", "file": "`mdl/backend/modelsdk/widget_write.go` (RegisterTypeDefaults NullFields Variable on Forms$MicroflowParameterMapping + Forms$NanoflowParameterMapping), `mdl/executor/cmd_alter_page.go` (convertASTAction seeds ParamScope, storedPageVariables, isSnippet from the mutator)", "insight": "**`mx diff base.mpr new.mpr out.mpr` reproduces Changes-panel crashes headless** — it runs the same MergeLib DifferenceComputer and prints the identical \"do not have the same properties\" line (exit 129). Base = a copy of the project before the write, made by the FIXED build so it has Studio Pro's key set. That turns a Windows-GUI-only symptom into a two-binary A/B on Linux. **The base must share the $ID** — the crash needs a paired object, which the identity carry supplies; a fresh element is only an add and never trips it, so a create-only repro passes on broken code. **Register the null slot, don't set it at the call site**: `Part.Set(nil)` still encodes as omitted, and there are three construction sites (action, nanoflow action, nanoflow data source) — NullFields covers all and any future one. **A builder constructed outside CREATE PAGE is the recurring hole for #1140-style classification**: INSERT/REPLACE already seeded the scope from the mutator, SET did not; grep for `&pageBuilder{` and check each seeds paramScope + localVariables + isSnippet. Measured on mxbuild 11.12.2: buggy mx diff exit 129 with the reported line, fixed exit 0; mx check 0 errors on both", "refs": ["mendixlabs/mxcli#1317", "mendixlabs/mxcli#1180", "mendixlabs/mxcli#1140"], "ce": ["CE1571"]} +{"area": "mdl/backend", "symptom": "alter page … { insert after tpOne { tabpage tpTwo … } } and insert into tabsMain { tabpage … } both fail: `failed to insert: failed to build widgets: failed to build widget tpTwo: tabpage must be a direct child of tabcontainer`; the only route to add a tab was rewriting the page with create or modify", "cause": "applyInsertWidgetMutator built each inserted node with buildWidgetV3, which refuses a lone tabpage (it is only built as a tabcontainer child); and even past the builder, InsertWidget refuses tab-page siblings by design (refuseTabPageSiblingEdit) because a Forms$TabPage is not a widget and lives in the control's TabPages list", "file": "mdl/executor/cmd_alter_page.go (applyInsertWidgetMutator, buildTabPagesFromAST); mdl/backend/pagemutator/tabpages.go (InsertTabPages)", "insight": "A non-widget child (list view template, DataGrid2 column, tab page) needs its own PageMutator method, not a relaxed check in InsertWidget: route by the node type in the executor before the generic builder runs, and let the mutator decide by the TARGET's $Type whether INTO (the container) or BEFORE/AFTER (a sibling of the same kind) is meant. Serialize by wrapping the new pages in a throwaway container and taking its child list back out — no new Deps method, and the shape is exactly CREATE PAGE's. Control that makes `mx check` meaningful: a Forms$TabPage placed in a tab page's Widgets makes mx unable to load the project (\"TabPage does not contain a constructor with a parameter of type TabPage\"), while the TabPages splice checks 0 errors on 11.14.0. Keep the control's DefaultPagePointer on insert; set it only when the control had no pages.", "refs": ["mendixlabs/mxcli#1215"], "date": "2026-10-07"} diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 935d6b987f..a793c7a09d 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -874,4 +874,8 @@ {"area": "mdl/executor", "date": "2026-10-07", "symptom": "Plain `mxcli check` (no -p) passed `set $X = $Y;` on an OBJECT variable (entity parameter, create, retrieve first, loop iterator, head) even after check --references and exec refused it; the build then failed with CE7247. The bug-test could not be a .fail.mdl because check-mdl runs plain check", "cause": "Plain check runs only the MDL0xx rule set (ValidateProgram -> ValidateMicroflow/ValidateNanoflow); validateFlowBody, where the first #1323 refusal lived, runs only under --references (validate.go) and in exec. A refusal added to one validator is invisible to the other path", "file": "`mdl/executor/validate_microflow_set_object.go` (`checkSetOnObjectVariable`, MDL-SET01), wired from `validate_microflow.go` (validate) and `validate_nanoflow.go`; `cmd_microflows_builder_validate.go` now reports only `assocObjectVars`", "insight": "Know which validator each entry point runs before adding a refusal: plain check = ValidateMicroflow; --references = that PLUS validateFlowBody; exec = the builder (plus execEnforcedMicroflowRules). Putting the rule in both check validators made --references print it twice; split by what each can see — the rule takes every object visible in the text, validateFlowBody only the association-typed ones that need the project. Measure every producer a rule judges, with the faulted build: all five plus a nanoflow gave CE7247 on 11.14.0, but a PARAMETER gives different wording, \"Parameter 'A' cannot be changed.\" — and that wording also fires for a PRIMITIVE parameter (`set $N = 1` on `$N: Integer`), a separate unrefused gap the object-only rule does not cover. Leave `find` out of the object producers: it is also the String function", "refs": ["mendixlabs/mxcli#1323", "ako/mxcli#1011"], "ce": ["CE7247"], "rules": ["MDL-SET01"]} {"area": "mdl-executor", "date": "2026-10-06", "symptom": "`create constant M.Flag (Type: Boolean, DefaultValue: true)` (or `True`) stores DefaultValue \"true\". Studio Pro stores \"True\"/\"False\"; its constant dialog shows a stored \"true\" as False while the runtime reads it as true, so the developer sees one value and the app runs with another. `mx check` is silent and `describe constant` prints `true` for both, so only the stored unit shows it. Quoted `'True'` was the workaround. `alter settings constant @M.Flag value true` (or 'true') wrote the configuration override the same way", "cause": "createConstant rendered the AST literal with fmt.Sprintf(\"%v\"), so a Go bool became Go's lowercase \"true\"; the quoted string passed through verbatim, which is why only 'True' was right. The settings path stored the visitor's token text verbatim and never looked at the constant's type", "file": "`mdl/executor/cmd_constants.go` (storedConstantDefault, used by the create and create-or-modify branches, and by `alterSettingsConstant` in `mdl/executor/cmd_settings.go` via settingsConstantType); test `mdl/executor/cmd_constant_boolean_default_test.go`; bug-test `mdl-examples/bug-tests/1321-boolean-constant-default-case.mdl`", "insight": "**`%v` on an AST literal is Go's spelling, not Mendix's**: any place that stringifies a parsed value into a stored property must normalise to the platform's form. The case-insensitive reader (formatDefaultValue's EqualFold) hid the writer's defect from describe, so a round-trip test could not catch it; assert on the value handed to the backend. Neither `mx check` nor the runtime distinguishes the two spellings — only Studio Pro's dialog does — so a build cannot verify this; the evidence is the reporter's Studio Pro 11.12.4 measurement plus the stored unit. Control: test written before the fix failed 5 of 6 spellings with `stored as \"true\"`/`\"false\"`, the quoted 'True' control passing; exec on a copy of testdata/pedapp then stores True/True/True/False. **Enumerate every write path for the value, not just the reported one**: the configuration override holds the same typed value and had the same defect; it needs the constant's type looked up (a String constant holding 'true' must stay lowercase). Control: the settings test failed 3 of 5 cases with `stored as \"true\"`/`\"FALSE\"` before the fix, the 'True' and String-constant controls passing; the stored Settings unit on a pedapp copy read 'true' for `value true` and `value 'true'` before, 'True' after, the String override 'true' both times", "refs": ["mendixlabs/mxcli#1321"]} {"area": "mdl/executor", "date": "2026-10-07", "symptom": "A button added to a snippet with `alter snippet … insert` (or replace) calling `call microflow M.F(P = $P)` with a snippet parameter fails mxbuild with CE0115 \"The arguments that are passed to microflow 'M.F' do not match the expected parameters and need to be refreshed\" on that button only; the same button in CREATE SNIPPET or via ALTER SNIPPET SET is clean", "cause": "`buildWidgetsFromAST` and `buildColumnSpecsFromAST` (mdl/executor/cmd_alter_page.go) seeded paramScope and localVariables from the mutator but not `isSnippet`, so `classifyFlowArgValue` returned kind \"parameter\" and the Forms$PageVariable named PageParameter instead of SnippetParameter", "file": "`mdl/executor/cmd_alter_page.go` (isSnippet from mutator.ContainerType() in both builders)", "insight": "**A pageBuilder built outside CREATE needs THREE scope fields, not two** — paramScope, localVariables and isSnippet; #1317 found the SET builder missing all three and this one missing the last. When adding a builder, copy the seeding from convertASTAction rather than from memory. **Unlike the Expression-form defect (#1140/#1317), mxbuild DOES catch the wrong slot** — CE0115 rather than CE1571 — so one script exercising CREATE, INSERT and SET on the same snippet with `mx check` is a complete A/B: only the broken path's button is named. Verified on 11.12.2: 1 error before, 0 after", "refs": ["mendixlabs/mxcli#1317", "mendixlabs/mxcli#1140"], "ce": ["CE0115"]} +{"area": "mdl/executor", "date": "2026-10-07", "symptom": "`create or modify microflow M.F () begin @excluded declare $Variable Boolean = false; return; end;` reports success but the activity is written enabled (ActionActivity Disabled=false); DESCRIBE prints no @excluded, so the round trip of any microflow with a disabled activity re-enables it. Same for every action statement (log, call, change, ...)", "cause": "`flowBuilder.mergeStatementAnnotations` copies ast.ActivityAnnotations into fb.pendingAnnotations field by field and never copied `Excluded`. The visitor set it, applyAnnotations honoured it (activity.Disabled = true), the backend writes Disabled, the describer emits @excluded — every layer was right except the hand-written merge between them", "file": "`mdl/executor/cmd_microflows_builder_annotations.go` (mergeStatementAnnotations)", "insight": "**Bisect by layer with the cheapest probe at each**: a visitor probe showed the AST carried Annotations, `bson dump` showed Disabled=false on disk, so the loss was between AST and write — and grepping `Disabled` showed exactly one setter, which was fine, pointing at its input. A field-by-field copy of a struct is a silent-drop site for every field added after it was written; the guard is a reflection test that sets every field and asserts it survives, with an explicit allowlist (Start, Invalid*, UnknownNames) for fields consumed elsewhere. Do not chase the backend or describer — both were already correct", "refs": ["mendixlabs/mxcli#1328"]} {"area": "mdl/executor", "date": "2026-10-07", "symptom": "`set $N = 1;` on a microflow/nanoflow/rule PARAMETER ($N: Integer or String) passes `mxcli check` and `exec`; mx check reports [CE7247] \"Parameter 'N' cannot be changed.\" at the Change variable activity", "cause": "Nothing modelled that a Change variable activity cannot target a parameter; the variable-kind tracking treated a primitive parameter like any declared primitive. Found while measuring MDL-SET01's object producers (#1323): the object parameter's CE7247 came back with a different wording, which was the tell that the restriction is on parameters, not on types", "file": "`mdl/executor/validate_microflow_set_object.go` (`checkSetOnObjectVariable` primitive-parameter branch, `setTargetViolations`); rules via `validate_program.go` (check) and `rule_validation.go` `validateRuleSetTargets` (exec, `cmd_rules_create.go`); MDL-SET01 added to `execEnforcedMicroflowRules` in `validate.go`", "insight": "When one measurement returns a different MESSAGE under the same CE code, test the cause the message names — here 'Parameter … cannot be changed' fired for an Integer too, a whole adjacent class. Measure the neighbours before writing the rule: list parameters (`set` = Change list Replace, `add`) and member changes on parameters BUILD, so the rule is 'any non-list parameter', not 'any parameter'. Rules never run ValidateMicroflow: plain check needs a ValidateProgram hook and exec a gate in cmd_rules_create; putting it in validateRule instead would double-print under --references, which runs both", "refs": ["mendixlabs/mxcli#1323"], "ce": ["CE7247"], "rules": ["MDL-SET01"]} +{"area":"mdl/executor","date":"2026-10-07","symptom":"`mxcli check script.mdl -p app.mpr --references` reported \"Check passed!\" for a declared type naming an entity that does not exist: a microflow/nanoflow/rule parameter or return type (`$p: System.Nope`, `$p: M.Nope`, `list of System.Nope`, `returns System.Nope`) and a page/snippet parameter (`params: ($x: System.Nope)`). Originally reported as `create association M.X_Y from System.Nope to M.Y` passing — that shape was already refused on main (#555).","cause":"validateWithContext resolved a flow's BODY (retrieve/create entity refs), an association's endpoints and an EXTENDS target, but never a document's SIGNATURE. exec refuses the user-module shapes and every page/snippet parameter after earlier statements are written; for a System entity in a flow signature it skips resolution (isBuiltinModuleEntity) and writes the dangling name.","file":"mdl/executor/validate_declared_types.go (flowSignatureErrors, documentParameterErrors); wired in validate.go for microflow/nanoflow/rule/page/snippet","insight":"Reproduce the reported shape on current main before fixing it: it had been fixed four days after the report, and the real hole was one statement-part over. Map a reference check by WHERE a name sits (body / endpoint / generalization / signature), not by module: System was never skipped wholesale — the virtual System domain model makes System.User resolve and System.Nope not, through the same buildEntityQualifiedNames as any module. A bare `Module.Name` type is ambiguous (entity or enumeration — `$d: System.DeviceType` is an enum), so accept either; only the explicit Enumeration(...) spelling is enum-only. A mock backend without the System DM must not make every System type 'missing' — absence of the whole module is not evidence about one name. Measure on the real path: the PedApp fixture through ValidateProgram + ValidateProgramWithWarnings (agreeCheck), not a MockBackend. Still open: `grant … on entity System.Nope to M.Role` and a grant to a non-existent module role pass --references; a java action parameter typed System.Nope also passes check (exec's verdict on it not measured).","refs":["#555","#610","#972"]} +{"area": "mdl-executor", "date": "2026-10-07", "refs": ["mendixlabs/mxcli#1324"], "symptom": "A widget directly inside data view `dvP` that reads `$dvP` OUTSIDE an action — a nested data view's `DataSource: microflow M.F(Gate = $dvP)`, `Visible: $dvP/Name != ''`, `Editable: …`, `DynamicClasses: …`, or a nested list's `database from M.E where [Name = $dvP/Name]` — passes `check` and `exec`, then `mx check` reports `[CE0117] \"Error(s) in expression.\"` at the widget (CE0161 \"Error(s) in XPath constraint.\" for the `where`)", "cause": "MDL-BUTTON02 (the #1324 fix) only looked at action arguments, though every slot evaluated in the widget's enclosing context has the same scope: a container's widget-name variable exists only one data container below it", "file": "`mdl/executor/validate_page_button_context.go` (`checkOwnContainerName` now takes the widget and walks action args, `GetDataSource().Args` / `.Where`, and `ownNameExprProps`)", "insight": "**A scope rule belongs to the context, not to the slot the report happened to use.** The follow-up question that settled it was one probe per slot with a control one data view deeper: five of five slots failed in the own context and all five controls built clean, so the rule is 'anything evaluated in this widget's enclosing context' — enumerate those slots rather than wait for one report each. Two things that would have wasted a build: (1) `Visible:`/`Editable:` arrive in the AST as `VisibleIf`/`EditableIf` strings (dump `w.Properties` before keying on what the author wrote), and (2) text-template parameters (`ContentParams ({1} = $dvP/Name)`) are already refused by MDL-WIDGET24 for an unrelated reason (a template parameter is an attribute name, not a variable path), so they are not a scope case at all. The XPath slot reports a different code (CE0161), which is why the message carries the code per slot. Control: stubbing the data-source and property slots fails the new test with `flagged \"\"`; the `$currentObject` rewrite of all five builds at 0 errors. Repro `mdl-examples/bug-tests/1324-own-data-container-name-outside-actions.fail.mdl`"} +{"area": "mdl/executor", "date": "2026-10-07", "symptom": "`describe structure depth 2|3` / `mxcli structure -d 3` never annotates a page with its data widgets (`Page M.P [DataView, …]`), on any project and with a full catalog — every page prints bare. Evora Factory Management: 0 of 54 pages annotated, although CATALOG.widgets holds their data views and grids.", "cause": "structurePages queried `widgets … where ParentWidget = ''`; widgets_data has never had a ParentWidget column (the tree position, added later, is ParentWidgetId + Depth). The query failed and the error was discarded (`if err == nil { … }`), so the annotation was dead code from the initial commit. Every other catalog query in cmd_structure.go swallowed errors the same way (`if err != nil || len(rows) == 0 { return }`).", "file": "`mdl/executor/cmd_structure.go` (`structureQuery`, `structurePages`, `queryCountByModule`, `shortWidgetType`), `mdl/executor/cmd_structure_page_widgets_test.go`", "insight": "A fixed SQL query against a table the builder owns can only fail through drift inside mxcli, never because of the user's project, so swallowing its error converts a schema mismatch into silent absence — the same shape as the depth-1 flow-count casing bug (#717), which a swallowed error also hid. Route such queries through one helper that returns the error; then a column rename fails the first test that runs the command against a real built catalog. That test must build the catalog with catalog.NewBuilder in FULL mode (SetFullMode(true)) over raw page BSON from GetRawUnitFunc — widgets_data is empty in fast mode, and a fast-mode test passes against the broken query because 'no widgets' and 'query failed' both print a bare page. Mock gotcha: the builder dereferences GetNavigation, whose mock default is (nil, nil), so stub it with an empty NavigationDocument. Semantics choice: 'top-level' cannot mean Depth = 0 — real pages wrap content in a layout grid, so a root filter lists almost nothing; list data widgets with no data-widget ancestor instead (walk ParentWidgetId). Measured on Evora after the fix: 45 of 54 pages annotated (181 of 212 with `all`), and the 9 bare pages have no data widget in CATALOG.widgets.", "refs": ["#717"]} diff --git a/.claude/skills/fix-issue/findings/mdl-other.jsonl b/.claude/skills/fix-issue/findings/mdl-other.jsonl index 7a9eaff339..1eee97cdd1 100644 --- a/.claude/skills/fix-issue/findings/mdl-other.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-other.jsonl @@ -99,3 +99,6 @@ {"area": "mdl/exprcheck", "date": "2026-10-04", "symptom": "ako/mxcli#969 item 1: `change $Log (Windrichting = if $Dir = 'NW' then E.NW else E.N)` on an enumeration attribute was refused with E001 (\"assigning an Enumeration attribute against a string literal\") for the 'NW' compared with a String; `find('|NW|NORTHWEST|', …)` inside the expression gave 3x E001 with fixes like `E.|`. exec runs check first, so a valid microflow could not be written", "cause": "parsePrimary ran checkStringLitVsSlot (E001) and the quoted-Boolean E002 on EVERY string literal while the slot path was set, so comparison operands, function arguments and if-conditions were judged as the slot's value", "file": "`mdl/exprcheck/parser.go` (checkValueLiterals)", "insight": "A slot constrains the VALUE, and the value is only the whole expression, a parenthesised one or a then/else result (recursively). Run slot-literal rules over the finished tree at those positions instead of during the parse, where the position is not yet known. Measured on 11.13.0: the comparison form builds clean, while a quoted 'NW' in a then-branch is CE0117 — so the branch rule is real and must keep firing. Note the WHOLE-literal form (`Wind = 'NW'`) builds clean because the writer rewrites it to the enum value; E001 there is stricter than mxbuild", "refs": ["#969"]} {"area": "mdl/exprcheck", "date": "2026-10-04", "symptom": "ako/mxcli#969 item 1 side finding: `change $Log (Wind = 'NW')` reported nothing in check when the enumeration and the entity were created in the same script; with them already stored it was E001", "cause": "TypeCheckProgram's CatalogReader is loaded from the catalog of the stored project only, so a script-declared attribute resolved to nothing, its kind was Unknown and every rule keyed on it stayed silent", "file": "`mdl/executor/typecheck.go` (declareScriptTypes), `mdl/exprcatalog/exprcatalog.go` (DeclareEnumeration, DeclareAttribute)", "insight": "Any catalog-backed check must overlay the script's own declarations, applied in statement order; Unknown is designed to suppress rules, so a missing overlay fails silent rather than loud. Test with the definitions in the SAME script as the use — the stored-project test passed all along", "refs": ["#969"]} {"area": "mdl/linter", "date": "2026-10-04", "symptom": "ako/mxcli#969 item 4: MPR006 warned that an empty container \"will crash at runtime\" (\"Did not expect an argument to be undefined\"), and the create-page skill told authors to pad every container with a dynamictext", "cause": "Claim from the initial commit with no measurement behind it", "file": "`mdl/linter/rules/empty_container.go`, `.claude/skills/mendix/create-page/SKILL.md`", "insight": "Measured on 11.13.0 (React client) with run --local --db-type hsqldb and a headless Chromium: a bare and a styled empty container render, the widgets after them are present, 0 console errors; mx check 0 errors. A runtime claim needs a runtime measurement — run --local + playwright is the layer (hsqldb avoids needing Postgres; the devcontainer needed the chromium shared libs apt-installed). Classic (Dojo) client not measured", "refs": ["#969"]} +{"area": "mdl/linter", "date": "2026-10-07", "symptom": "upstream mendixlabs/mxcli#1217: \"Lint CONV011 (NoCommitInLoop) misses CHANGE … COMMIT (and CREATE … COMMIT) inside a loop; only a separate COMMIT activity is flagged\". `change $T (\"Done\" = true) commit;` in a loop passed lint and `check`; `change …; commit $T;` was flagged. Studio Pro's recommender flags both (MXP004).", "cause": "CONV011 matched only `*microflows.CommitObjectsAction`; the Commit property of CreateObjectAction / ChangeObjectAction was never consulted. MDL-PERF01 (check-time twin, #1186) pinned its boundary to CONV011's, so it inherited the same gap by design.", "file": "`mdl/linter/rules/conv_loop_commit.go` (`committingActionKind`), `mdl/executor/validate_commit_in_loop.go` (ChangeObjectStmt/CreateObjectStmt cases); tests `conv_loop_commit_test.go`, `validate_commit_in_loop_test.go`; example `mdl-examples/bug-tests/1217-commit-clause-in-loop.mdl`", "insight": "**A rule that detects a database effect must enumerate every action that HAS the effect, not the action NAMED after it.** Commit is a property on create/change as well as an activity of its own; grep the action types for a `Commit` field before trusting a commit rule's coverage. Any value other than `No` commits — YesWithoutEvents skips handlers, not the round trip. When two rules share a pinned boundary (CONV011 ↔ MDL-PERF01), a coverage fix lands in both in the same change or they drift. Verified end to end: exec the repro into testdata/expr-checker/minimal.mpr and `lint -r CONV011` — reverted build flags 1 of 3 (only the separate commit), fixed build 3 of 3, list-commit-after-loop control quiet in both.", "refs": ["mendixlabs/mxcli#1217", "mendixlabs/mxcli#1186"], "rules": ["CONV011", "MDL-PERF01"]} +{"area": "mdl/exprcheck", "date": "2026-10-07", "symptom": "mendixlabs/mxcli#1216: `declare $D DateTime = parseDateTimeUTC($Text, 'yyyy-MM-dd', empty);` — `mxcli check -p` reports `parseDateTimeUTC() expects 2 argument(s), got 3. [E006]` and exec refuses, while mx check on 11.14.0 reports 0 errors; only --no-check writes it", "cause": "funcTable listed the parse functions without their default-value overloads: parseDateTime/parseDateTimeUTC(value, format [, default]), parseInteger(value [, default]), parseDecimal(value [, format [, default]]) were all fixed at their minimum arity", "file": "`mdl/exprcheck/func_checker.go` (funcTable), test `mdl/exprcheck/parse_default_arity_test.go`, `mdl-examples/bug-tests/1216-parse-datetime-default-value.mdl`", "insight": "funcTable arities are a transcription, so widen each one only by measurement, and measure the siblings the reporter says 'presumably' — two of them (parseInteger/parseDecimal) had the same false E006, but parseBoolean($s, false) is CE0117 and stays 1-arg. Isolate each case in its own project copy: mx check names the activity by its caption ('Create Date and time variable'), so several cases in one project are indistinguishable. Plausible wrong turn: a default of currentDateTime() fails CE0117, which reads as 'the 3-arg UTC form is invalid' — it is currentDateTime() itself, rejected on its own in a microflow; vary the default ($var, empty) before concluding. E006 only fires with -p, so a project-less `mxcli check` of the repro passes and proves nothing.", "ce": ["CE0117"], "rules": ["E006"]} +{"area": "mdl/exprcheck", "date": "2026-10-07", "symptom": "`declare $D DateTime = currentDateTime();` passes `mxcli check` and `exec`, then mx check on 11.14.0 fails `[error] [CE0117] \"Error(s) in expression.\" at Create variable activity 'Create Date and time variable'` — in a microflow and a nanoflow alike", "cause": "funcTable listed `currentDateTime` as a zero-argument built-in. Mendix has no such function; the current time is the `[%CurrentDateTime%]` token. funcTable is MDL044's sole allow-list, so the entry silenced the one rule that would have caught it", "file": "`mdl/exprcheck/func_checker.go` (entry removed), `mdl/exprcheck/unknown_funcs.go` (tokenFuncs → FuncRef.Token), `mdl/executor/validate_microflow.go` (MDL044 hint), bug tests `current-datetime-function.fail.mdl` / `current-datetime-token.mdl`", "insight": "Found by accident while measuring #1216: a default of currentDateTime() made parseDateTime look like it rejected a third argument. Before blaming the outer construct, build the inner expression on its own. The removal alone would give a useless hint — nearestFunc offers a spelling match, and the right answer is a token rather than a function — so name the token in the hint. Nothing in the repo emitted or recommended currentDateTime() (every example uses the token), which suggests the entry came from transcription, like the year()/month()/trunc() entries before it. The remaining unverified extraction names (dayOfYear, hour, …) are the same risk.", "ce": ["CE0117"], "rules": ["MDL044"]} diff --git a/.claude/skills/fix-issue/findings/mdl-visitor.jsonl b/.claude/skills/fix-issue/findings/mdl-visitor.jsonl index 6e524f0ee1..222d316670 100644 --- a/.claude/skills/fix-issue/findings/mdl-visitor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-visitor.jsonl @@ -57,3 +57,4 @@ {"area": "mdl/visitor", "date": "2026-10-03", "symptom": "`mxcli -p app.mpr -c \"show project version\"` (also `list project version`, `show project roles`) panics: nil pointer dereference in recordShowSingleThing at visitor_verbs.go:39", "cause": "`show project` continues only with `security`; ANTLR recovers by dropping the stray word, leaving a ShowStatementContext with PROJECT and no SECURITY token, and the MDL-DEPR090 listener called stmt.SECURITY().GetSymbol() unguarded", "file": "`mdl/visitor/visitor_verbs.go` (recordShowSingleThing), hint in `mdl/visitor/visitor.go` enhanceErrorMessage", "insight": "Deprecation/gate listeners run on error-recovered trees too: any `ctx.X().GetSymbol()` on a token the rule makes mandatory can be nil after recovery. `show project` alone conjures the missing token (no panic) — a stray extra word is what triggers it, so test the 'one wrong word' form, not the truncated one", "refs": []} {"area": "mdl/visitor", "date": "2026-10-05", "symptom": "A blank line inside a /** */ documentation block is dropped for entities, attributes and microflows: the stored documentation collapses to \"First paragraph.\\nSecond paragraph.\", so Studio Pro's \"\\n\\n\" comes back as \"\\n\" on describe -> exec; mx check clean", "cause": "extractDocComment (and its copy extractDocCommentText for REST) skipped every line that was empty after stripping the leading `*`, treating interior paragraph breaks like the framing `/**` / ` */` lines", "file": "`mdl/visitor/visitor_helpers.go` (extractDocComment), `mdl/visitor/visitor_rest.go` (now delegates), `mdl/executor/documentation_carry.go` (docCommentNormalForm)", "insight": "Only the blank lines at the block's edges are framing; trim those, keep interior ones. docCommentNormalForm in the executor encodes the visitor's rule a second time (the #731 'same text, different spelling' comparison) - change one and the other must follow, or describe output of two-paragraph prose compares unequal to itself and every re-run rewrites the unit. The bug was invisible to describe/exec idempotence tests because both sides used the collapsed form; only a stored \"\\n\\n\" written by Studio Pro exposes it", "refs": ["mendixlabs/mxcli#1300", "#731"]} {"area": "mdl/visitor", "date": "2026-10-06", "symptom": "`send email` first shipped as keyword clauses (from/to/subject/host/port …) copied from the pre-ADR-0013 `call rest service`; the clause form required To and Subject", "cause": "Syntax copied from the nearest existing activity rather than from the rule: ADR-0013 (activity settings are one ( Key: value ) list) had been decided in a parallel PR (ako/mxcli#1006) and the REST activity was its first migration. The required set was a guess from Studio Pro's dialog, not a measurement", "file": "mdl/visitor/visitor_send_email_settings.go", "insight": "For a NEW activity, read design-mdl-syntax 'Activity statements' and the REST settings implementation before copying any existing statement — the nearest neighbour may be the very thing being migrated. A new activity needs no clause form and no MDL-DEPR entry; the clause spelling becomes a plain parse error (TestSendEmailHasNoClauseForm). Settings keys are identifiers, so only the verb (EMAIL) is a new lexer token — the clause draft had added eight. Measure 'required' with one omitted setting per activity through mxbuild before writing the visitor's required list: 11.15.0-rc.4 gives CE0166 for From, Host, Port and for 'To, Cc, or Bcc' (one recipient of the three, not To), and NO error for a missing Subject — two of the five guessed requirements were wrong. Keep the metamodel's ConnectionTimeout (ms) rather than REST's Timeout (s): one key name must not carry two units. Proof of round trip on Studio Pro-authored input: describe ako/TestApp Email.EmailMF, exec the statement under a new name, and the two SendEmailAction dumps are identical line for line.", "refs": ["mendixlabs/mxcli#1315", "ako/mxcli#1006"], "ce": ["CE0166"], "rules": ["MDL-EMAIL02", "MDL-EMAIL03"]} +{"area": "mdl/visitor", "date": "2026-10-07", "symptom": "`create or modify published rest service M.Api (Path: '\u2026', Authentication: microflow M.AuthMf) { \u2026 }` crashed `check` and `exec` with `panic: runtime error: invalid memory address or nil pointer dereference` in visitor.unquoteStringLit called from ExitCreatePublishedRestServiceStatement (visitor_rest.go). `Authentication: Basic` and `Folder: microflow M.F` did the same. Reported as mendixlabs/mxcli#1331.", "cause": "publishedRestProperty is `identifierOrKeyword COLON STRING_LITERAL`. Build() walks a failed parse on purpose, so under error recovery the property context exists with no STRING_LITERAL child, and unquoteStringLit called GetText() on the nil interface. The second instance of mendixlabs/mxcli#1023's class, after that fix guarded one call site.", "fix": "unquoteStringLit returns \"\" for a nil node, covering all ~270 call sites at once. The author now sees the syntax error the listener had already recorded: `mismatched input 'microflow' expecting STRING_LITERAL`.", "file": "mdl/visitor/visitor_string_escapes.go", "insight": "#1023's insight said to grep for unguarded `STRING_LITERAL().GetText()`, but most sites go through unquoteStringLit, which that grep never finds. Guard the shared helper once: a nil there always means a parse that already failed, so \"\" can never reach exec, and per-site guards miss the next rule. The issue's second half (no way to set authentication) is a capability gap, not this bug: a quoted `Authentication: 'basic'` already warns MDL-V1-PROP as an unknown property, so nothing was dropped silently.", "refs": ["mendixlabs/mxcli#1331", "mendixlabs/mxcli#1023"]} diff --git a/.claude/skills/mendix/alter-page/SKILL.md b/.claude/skills/mendix/alter-page/SKILL.md index 6a029c0583..a6e7c35e99 100644 --- a/.claude/skills/mendix/alter-page/SKILL.md +++ b/.claude/skills/mendix/alter-page/SKILL.md @@ -292,8 +292,14 @@ Inserted widgets use the same syntax as `create page`. Multiple widgets can be i only way to fill an **empty** container, and handy for adding to a container/dataview without needing a sibling to anchor to. Widgets inserted into a dataview take that dataview's entity as their context. Supported on simple containers (container, -dataview, groupbox, scroll-container region); for a layout grid or tab container, -insert relative to a widget inside the target column/tab instead. +dataview, groupbox, tab page, scroll-container region); for a layout grid, insert +relative to a widget inside the target column instead. + +**Adding a tab:** `insert into { tabpage … }` appends a tab page, and +`insert after|before { tabpage … }` places it next to that sibling. A tab +page cannot go next to an ordinary widget or inside another tab page, and one insert +cannot mix tab pages and widgets. Dropping or reordering tab pages still needs +`create or modify page`. **The context comes from the nearest enclosing data source, whatever kind it is** — a database or association source, a microflow/nanoflow source (the entity is diff --git a/.claude/skills/mendix/cheatsheet-errors/SKILL.md b/.claude/skills/mendix/cheatsheet-errors/SKILL.md index 002af66d67..e7c8e4b255 100644 --- a/.claude/skills/mendix/cheatsheet-errors/SKILL.md +++ b/.claude/skills/mendix/cheatsheet-errors/SKILL.md @@ -265,7 +265,7 @@ Run with `-p` for the fullest coverage. | CE0104 | Action activity is unreachable | Code after RETURN | | CE0105 | Must end with end event | Missing RETURN | | CE0117 | Error in expression | Unqualified association path | -| CE0117 | …on a button inside a data container | The button passes the container it sits in by its widget name (`$dvGate` inside `dvGate`). That name is a variable only for containers nested below it; use `$currentObject` there. MDL-BUTTON02 | +| CE0117 | …on a button inside a data container | The button passes the container it sits in by its widget name (`$dvGate` inside `dvGate`). That name is a variable only for containers nested below it; use `$currentObject` there. Same for data-source arguments, `Visible:`, `Editable:`, `DynamicClasses:`, and (as CE0161) a nested list's XPath `where`. MDL-BUTTON02 | | CE1571 | No argument selected for parameter | A microflow/nanoflow call with a parameter nothing fills — as a `datasource:` **or** an `action:`. Give it an argument (`action: call nanoflow M.NF(P = $value)`), or nest the widget in a data container of the parameter's type. `check -p` reports both | | CE1571 | …in a control bar | A control bar is **not** row-scoped, so the grid's row does not fill it: pass the grid's selection (`$dgOrders`, with `Selection:` set) or move the widget into a column. `$currentObject` there is MDL-BUTTON01 | | CE1834 | The 'Page' property is required | Workflow user task without a `page` — `check` flags MDL-WF01 | diff --git a/.claude/skills/mendix/create-page/reference/widgets.md b/.claude/skills/mendix/create-page/reference/widgets.md index 24d650928d..123aa12e47 100644 --- a/.claude/skills/mendix/create-page/reference/widgets.md +++ b/.claude/skills/mendix/create-page/reference/widgets.md @@ -1073,7 +1073,10 @@ view `dvOrder`'s object from a data view, list or grid nested inside it. In the container's own context — directly inside it, through plain containers, or in the control bar of a grid inside it — the object is `$currentObject`, and `$dvOrder` is **CE0117** "Error(s) in expression." (`mxcli check` reports -MDL-BUTTON02). The same goes for a list view, gallery or grid read by its own +MDL-BUTTON02). That holds for every slot evaluated there, not only action +arguments: a nested widget's microflow data-source arguments, `Visible:`, +`Editable:` and `DynamicClasses:` (CE0117), and a nested list's XPath `where` +(**CE0161**). The same goes for a list view, gallery or grid read by its own name from its item or row. A grid's own name *is* valid from its control bar — that is the selection, above (mendixlabs/mxcli#1324). diff --git a/CHANGELOG.md b/CHANGELOG.md index fd508e07e2..cf9f1a5038 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added +- **A published REST service's authentication** (mendixlabs/mxcli#1331) — `create [or modify] published rest service M.Api (…, Authentication: (basic, session, microflow M.Authenticate))` sets Studio Pro's **Requires authentication** and its methods: `basic` (username and password), `session` (active session), `microflow M.F` (custom); `Authentication: none` is "Requires authentication: No". The methods are stored in the order written, which is how Studio Pro stores the ones ticked, and `describe` prints them in the stored order (it omits `none`). Left out, `create or modify` and `alter` keep the stored setting, as before. `alter published rest service M.Api set ( Key: value, … )` now takes create's property list (`Path`, `Version`, `ServiceName`, `Authentication`); `set Key = '…'` still works. The authentication microflow must return `System.User` and take only a `System.HttpRequest` and/or `System.HttpResponse` (Mendix: CE0334, CE0336, measured with `mx check` on 11.14.0); `exec` and `check --references` refuse anything else as **MDL-REST04**. Executing the `describe` output of each of ako/TestApp's Studio Pro services writes nothing. - **`send email` — the built-in Send Email activity (Mendix 11.13+, beta)** — `send email ( From: …, To: …, Subject: 'Order {1}' with ({1} = …), Body: template '…', HtmlBody: template '…', Headers: ('X-Name': 'value'), Attachment: $Doc, Host: …, Port: …, SecurityType: ssl, CheckServerIdentity: true, ConnectionTimeout: 30000, Authentication: basic (Username: …, Password: …) ) [on error …];` creates a `Microflows$SendEmailAction`, which sends SMTP mail without the Email Connector module. The settings are one property list (ADR-0013), keyed by the metamodel's names; an unknown, repeated or misshapen key is an error, and so is a missing From, Host, Port or recipient (mxbuild: CE0166). `describe microflow` renders the activity instead of `-- Unsupported action: Microflows$SendEmailAction`, and the description re-executes to the same activity. `check` type-checks its expressions (E009: String addresses, host and credentials, Integer/Long port, String template parameters — what mxbuild reports as CE9528/CE0117), and warns on a server-identity check without SSL and on header names Studio Pro would refuse (MDL-EMAIL02/03). A 10.x–11.12 project is refused; a stored activity MDL cannot restate (authentication document, pre-11.13 message) still describes as unsupported rather than being rewritten smaller. `mxcli syntax microflow.send-email`. (mendixlabs/mxcli#1315) - **`mxcli playwright check` — a text verdict for pages of a running app, in one call** — `mxcli playwright check /p/a /p/b -p app.mpr` loads each page in one headless browser and prints the verdict `run --page-check` prints (title, heading, rows, visible text, error banners, console errors) plus failed same-origin requests and the HTTP status, then `OK n page(s)` or `FAIL k of n page(s)`. **Exit status** 0 all passed, 1 a page failed (HTTP error, sign-in form or a 401 instead of the page, an error banner or error dialog, a console error, a failed request, a failed assertion), 2 the check could not run. It signs in when needed — `--user/--password`, `--role R` (the project's demo user with that user role), or with only `-p` a demo user — saves the session under `.mxcli/playwright-check/`, reuses it on the next check and renews it when the runtime has restarted. `--assert-text`, `--assert-count 'SELECTOR>=N'`, and `--screenshot out.png`, which prints the path and never the image. It replaces the hand-written playwright-cli login/goto/sleep/eval/screenshot sequences that were about a sixth of the tool calls in a measured app-building session, each ending in a PNG read; the `test-app` and `verify-in-runtime` skills and `/mendix:test` now route "check a page" to it. - **`mxcli run --local` runs as a background service** — `--detach` starts the warm loop in its own session and returns once the app serves (`running: http://127.0.0.1:8080/ (pid N, watch, log .mxcli/run.log)`), or prints the boot failure and the last log lines and exits 1. `mxcli run status` prints one line (URL, uptime, last build and how it was applied, a pending change; a dead pid is reported as stopped). `mxcli run wait` blocks until the model as it is now has been applied — `applied: build #3 via reload in 1.4s` — or failed, with the CE codes; it is race-free (a change already applied when it starts is reported at once), so `mxcli exec x.mdl && mxcli run wait` is one honest call; `--ready` waits for the boot. `mxcli run stop` shuts the run down and then sweeps its process session, so no orphaned runtimelauncher or mxbuild JVM is left, even from a run killed with `-9`. `mxcli run restart` stops and detaches again with the same arguments. Exit codes: 0 ok, 1 failed, 2 timeout, 3 not running, 4 starting. A second run for a project that already has one is refused. These replace the `nohup` / `until grep` / `pkill` loops that took about a quarter of all tool calls in measured agent sessions; the run-local and run-app skills and the generated CLAUDE.md now teach them. @@ -24,7 +25,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed - **`set $Param = …` on a parameter is refused** — a Change variable cannot target a parameter, and mxbuild rejects it with CE7247 "Parameter 'N' cannot be changed." (measured on 11.14.0 for Integer and String parameters in a microflow, a nanoflow and a rule). `check` and `exec` passed it. MDL-SET01 now refuses it for every parameter but a list (`set` on a list parameter is a Change list Replace, which builds), and `exec` enforces the rule. Copy the parameter into a variable first: `declare $Value Integer = $N;`. -- **`check` refuses a button that passes its own data container by widget name** (mendixlabs/mxcli#1324) — `actionbutton btnOwn (Action: call microflow M.F(Gate = $dvGate))` directly inside data view `dvGate` passed `check --references` and `exec`, then `mx check` reported `[CE0117] "Error(s) in expression." at Action button 'btnOwn'`. A data container's name is a variable only for the containers nested below it; in its own context the object is `$currentObject`. Reported as **MDL-BUTTON02** (error, so `exec` refuses it), for data views, list views, galleries and data grids alike, including attribute paths (`$dvGate/Name`) and a control bar inside the container. A grid's own name from its control bar (the selection) and an enclosing container's name from a nested one are not flagged. The flagged set matches mxbuild 11.14.0's CE0117s widget for widget on a ten-button probe page. +- **`check` refuses a button that passes its own data container by widget name** (mendixlabs/mxcli#1324) — `actionbutton btnOwn (Action: call microflow M.F(Gate = $dvGate))` directly inside data view `dvGate` passed `check --references` and `exec`, then `mx check` reported `[CE0117] "Error(s) in expression." at Action button 'btnOwn'`. A data container's name is a variable only for the containers nested below it; in its own context the object is `$currentObject`. Reported as **MDL-BUTTON02** (error, so `exec` refuses it), for data views, list views, galleries and data grids alike, including attribute paths (`$dvGate/Name`) and a control bar inside the container. The rule covers every slot evaluated in that context, not only action arguments: a nested widget's microflow data-source arguments, `Visible:`, `Editable:` and `DynamicClasses:` (CE0117), and a nested list's XPath `where` (CE0161). A grid's own name from its control bar (the selection) and an enclosing container's name from a nested one are not flagged. The flagged set matches mxbuild 11.14.0's CE0117s widget for widget on a ten-button probe page. - **`set $Obj = …` on an object variable is refused** (mendixlabs/mxcli#1323) — with both variables single objects (e.g. a Reference retrieved from its FROM entity), `set $Cursor = $Next;` passed `check --references` and `exec` and was written as a Change variable action, which mxbuild refuses with CE7247 "Variable 'Cursor' does not have a primitive type". Mendix has no action that reassigns an object variable, so `check` (MDL-SET01, for the objects it can see without a project), `check --references` and `exec` now refuse it and name the alternatives (a sub-microflow that returns the next object, `change $Obj (…)`). `check --references` now types a Reference retrieve from its FROM entity as the object it is. `set` on a list variable stays a Change list Replace (ako/mxcli#949). - **`RENAME JAVA ACTION` renames the Java class, not just the file** — renaming `M.JA_Old` to `JA_New` moved the source to `JA_New.java` but left `public class JA_Old`, its constructor and its `toString` inside it, which javac rejects (`class JA_Old is public, should be declared in a file named JA_Old.java`). A full build hid it, because mxbuild regenerates the stub in its own copy; `run --local --watch` hot reload and IDEs compile the file on disk. Those three places now follow the rename exactly as mxbuild rewrites them; user code and extra code are left as written. Applies to `mxcli rename java-action` too. - **`run --local --watch` starts on Mendix 10.24 and 11.6** — it waited the full web-client timeout (5 minutes) for a bundle that had finished in seconds, then failed with `web client watcher timed out`. The rollup runner those versions ship reports its status as `{"code":"SUCCESS"}` rather than the modern-web-bundler protocol mxcli was reading; both are now understood, including that runner's error reports. diff --git a/cmd/mxcli/syntax/features_integration.go b/cmd/mxcli/syntax/features_integration.go index db93445891..6b67f0c38c 100644 --- a/cmd/mxcli/syntax/features_integration.go +++ b/cmd/mxcli/syntax/features_integration.go @@ -339,9 +339,10 @@ func init() { "create published rest", "publish rest", "rest resource", "rest operation", "microflow", "path parameter", "query parameter", "body parameter", "import mapping", "export mapping", "commit", - "grant access", "revoke access", + "grant access", "revoke access", "authentication", "custom authentication", + "basic authentication", "active session", "authentication microflow", }, - Syntax: "CREATE [OR MODIFY] PUBLISHED REST SERVICE Module.Name (\n Path: 'rest/api/v1',\n Version: '1.0.0',\n ServiceName: 'My API'\n)\n{\n RESOURCE 'name' {\n GET '' MICROFLOW Module.GetAll;\n GET '{id}' MICROFLOW Module.GetById;\n POST '' MICROFLOW Module.Create\n [IMPORT MAPPING Module.IMM] [EXPORT MAPPING Module.EMM]\n [COMMIT Yes | YesWithoutEvents | No]; -- no COMMIT clause: Yes\n }\n};\n\n" + + Syntax: "CREATE [OR MODIFY] PUBLISHED REST SERVICE Module.Name (\n Path: 'rest/api/v1',\n Version: '1.0.0',\n ServiceName: 'My API',\n [Authentication: none | ( basic, session, microflow Module.Authenticate )]\n)\n{\n RESOURCE 'name' {\n GET '' MICROFLOW Module.GetAll;\n GET '{id}' MICROFLOW Module.GetById;\n POST '' MICROFLOW Module.Create\n [IMPORT MAPPING Module.IMM] [EXPORT MAPPING Module.EMM]\n [COMMIT Yes | YesWithoutEvents | No]; -- no COMMIT clause: Yes\n }\n};\n\n" + "-- Operation parameters come from the microflow, as Studio Pro derives them:\n" + "-- a parameter named in the path ('{id}') -> path parameter\n" + "-- an object or a list -> the body\n" + @@ -349,8 +350,17 @@ func init() { "-- anything else -> query parameter\n" + "-- each with the microflow parameter's type. Create the microflow first.\n" + "-- A header parameter, a renamed one or a description set in Studio Pro has\n" + - "-- no MDL spelling: describe notes it, CREATE OR MODIFY / ALTER keep it.\n\nALTER PUBLISHED REST SERVICE Module.Name SET Version = '2.0.0';\nALTER PUBLISHED REST SERVICE Module.Name ADD RESOURCE 'items' { ... };\nALTER PUBLISHED REST SERVICE Module.Name DROP RESOURCE 'legacy';\nDROP PUBLISHED REST SERVICE Module.Name;", - Example: "mdl 1;\nCREATE PUBLISHED REST SERVICE Module.OrderAPI (\n Path: 'rest/orders/v1',\n Version: '1.0.0',\n ServiceName: 'Order API'\n)\n{\n RESOURCE 'orders' {\n GET '' MICROFLOW Module.GetAllOrders;\n GET '{id}' MICROFLOW Module.GetOrderById;\n POST '' MICROFLOW Module.CreateOrder;\n DELETE '{id}' MICROFLOW Module.DeleteOrder;\n }\n};\n\nGRANT ACCESS ON PUBLISHED REST SERVICE Module.OrderAPI\n TO Module.User, Module.Admin;", + "-- no MDL spelling: describe notes it, CREATE OR MODIFY / ALTER keep it.\n\n" + + "-- Authentication: the methods in the order given (Studio Pro keeps that\n" + + "-- order): basic = username and password, session = active session,\n" + + "-- microflow = custom. `none` is \"Requires authentication: No\". Left out,\n" + + "-- CREATE OR MODIFY and ALTER keep the stored setting; a new service has none.\n" + + "-- The microflow returns System.User (empty: not authenticated) and takes\n" + + "-- only a System.HttpRequest and/or System.HttpResponse (MDL-REST04).\n" + + "-- With authentication on, grant at least one module role (CE0338);\n" + + "-- custom authentication also needs app security on (CE6600).\n\n" + + "ALTER PUBLISHED REST SERVICE Module.Name SET ( Version: '2.0.0', Authentication: (session) );\nALTER PUBLISHED REST SERVICE Module.Name SET Version = '2.0.0';\nALTER PUBLISHED REST SERVICE Module.Name ADD RESOURCE 'items' { ... };\nALTER PUBLISHED REST SERVICE Module.Name DROP RESOURCE 'legacy';\nDROP PUBLISHED REST SERVICE Module.Name;", + Example: "mdl 1;\nCREATE PUBLISHED REST SERVICE Module.OrderAPI (\n Path: 'rest/orders/v1',\n Version: '1.0.0',\n ServiceName: 'Order API',\n Authentication: (basic, session)\n)\n{\n RESOURCE 'orders' {\n GET '' MICROFLOW Module.GetAllOrders;\n GET '{id}' MICROFLOW Module.GetOrderById;\n POST '' MICROFLOW Module.CreateOrder;\n DELETE '{id}' MICROFLOW Module.DeleteOrder;\n }\n};\n\nGRANT ACCESS ON PUBLISHED REST SERVICE Module.OrderAPI\n TO Module.User, Module.Admin;", SeeAlso: []string{"rest", "rest.consumed"}, }) diff --git a/docs-site/src/examples/rest-integration.md b/docs-site/src/examples/rest-integration.md index 097dfc9d9c..98be5f6082 100644 --- a/docs-site/src/examples/rest-integration.md +++ b/docs-site/src/examples/rest-integration.md @@ -357,6 +357,51 @@ CREATE OR MODIFY PUBLISHED REST SERVICE Module.OrderAPI ( }; ``` +### Authentication + +`Authentication` sets how a caller authenticates — Studio Pro's **Requires +authentication** and its methods: + +```sql +mdl 1; +CREATE OR MODIFY PUBLISHED REST SERVICE Module.OrderAPI ( + Path: 'rest/orders/v1', + Version: '1.0.0', + ServiceName: 'Order API', + Authentication: (basic, session, microflow Module.PRS_Authenticate) +) +{ + RESOURCE 'orders' { + GET '' MICROFLOW Module.PRS_GetAllOrders; + } +}; +GRANT ACCESS ON PUBLISHED REST SERVICE Module.OrderAPI TO Module.User; + +ALTER PUBLISHED REST SERVICE Module.OrderAPI SET (Authentication: (session)); +ALTER PUBLISHED REST SERVICE Module.OrderAPI SET (Authentication: none); +``` + +| Method | Studio Pro | +|---|---| +| `basic` | Username and password | +| `session` | Active session | +| `microflow Module.Name` | Custom, with that authentication microflow | +| `none` (instead of a list) | Requires authentication: No | + +- The methods are stored **in the order written**, as Studio Pro stores them in + the order they were ticked; `describe` prints the stored order. +- **Left out**, `CREATE OR MODIFY` and `ALTER` keep the service's stored + setting. A new service without it requires no authentication. `describe` + does not print `none`. +- The **authentication microflow** returns `System.User` — empty means "not + authenticated" — and its parameters can only be a `System.HttpRequest` and a + `System.HttpResponse`, matched by type (none at all is fine). `exec` and + `check --references` refuse anything else as **MDL-REST04** (Mendix: CE0334, + CE0336). +- With authentication on, the service needs at least one allowed module role + (`GRANT ACCESS …`), or `mx check` reports CE0338. Custom authentication also + needs app security on (CE6600). Roles without authentication are accepted. + ### Multiple Resources ```sql diff --git a/docs-site/src/reference/page/alter-page.md b/docs-site/src/reference/page/alter-page.md index 3cc59067f9..a3e8457b01 100644 --- a/docs-site/src/reference/page/alter-page.md +++ b/docs-site/src/reference/page/alter-page.md @@ -89,7 +89,27 @@ Inserts new widgets immediately before or after a named widget within its parent Appends new widgets as the **last children** of a named container. This is the only way to fill an **empty** container, and the natural way to add a widget to a container or data view without needing a sibling to anchor to. Widgets inserted into a data view take that data view's entity as their context. -Supported on simple containers (container/DivContainer, data view, group box, scroll-container region). A layout grid (rows/columns) and tab container have no single child list — insert relative to a widget inside the target column or tab instead. +Supported on simple containers (container/DivContainer, data view, group box, tab page, scroll-container region). A layout grid (rows/columns) has no single child list — insert relative to a widget inside the target column instead. A tab container's children are tab pages; see below. + +### Adding Tab Pages + +A tab container holds tab pages, and only tab pages. `INSERT INTO` the tab +container appends one; `INSERT BEFORE` / `INSERT AFTER` a tab page places it next +to that sibling. The container keeps its default tab. + +```sql +mdl 1; +alter page ModT.TabsPage { + insert after tpOne { tabpage tpTwo (Caption: 'Two') { dynamictext txtTwo (Content: 'two') } } +}; +alter page ModT.TabsPage { + insert into tabsMain { tabpage tpThree (Caption: 'Three') { dynamictext txtThree (Content: 'three') } } +}; +``` + +A tab page cannot go next to an ordinary widget or inside another tab page, and +one `INSERT` cannot mix tab pages with widgets. Dropping, replacing or reordering +tab pages still needs `CREATE OR MODIFY PAGE`. ### DROP diff --git a/docs-site/src/tools/builtin-rules.md b/docs-site/src/tools/builtin-rules.md index 73b8629ff3..2789a9d7c9 100644 --- a/docs-site/src/tools/builtin-rules.md +++ b/docs-site/src/tools/builtin-rules.md @@ -34,7 +34,7 @@ The **lint rules** below run with `mxcli lint`. There is also a separate group o | Rule | Description | |------|-------------| -| **CONV011** | No commit in loop -- Detects COMMIT statements inside LOOP blocks (performance anti-pattern) | +| **CONV011** | No commit in loop -- Detects COMMIT statements, and CREATE / CHANGE with a COMMIT clause, inside LOOP blocks (performance anti-pattern) | | **CONV012** | Exclusive split captions -- Checks that decision branches have meaningful captions | | **CONV013** | Error handling on external calls -- Ensures external service calls have error handling | | **CONV014** | No continue error handling -- Warns against using CONTINUE error handling without logging | diff --git a/docs-site/src/tutorial/describe-structure.md b/docs-site/src/tutorial/describe-structure.md index 5b486066b2..75409ae12c 100644 --- a/docs-site/src/tutorial/describe-structure.md +++ b/docs-site/src/tutorial/describe-structure.md @@ -111,6 +111,8 @@ MyFirstModule Depth 3 is verbose but gives you the most complete picture without running individual DESCRIBE commands. +At depths 2 and 3 each page also lists its outermost data widgets, the ones that set up a data context, for example `Page MyFirstModule.Customer_Edit [DataView]`. A data widget nested inside another one is not listed. The widget index is built only by `REFRESH CATALOG FULL`, so on a default catalog pages print without the list. + ## Filtering by module Use `IN` to show only a single module: diff --git a/docs/11-proposals/PROPOSAL_published_rest_authentication.md b/docs/11-proposals/PROPOSAL_published_rest_authentication.md new file mode 100644 index 0000000000..082dd71646 --- /dev/null +++ b/docs/11-proposals/PROPOSAL_published_rest_authentication.md @@ -0,0 +1,196 @@ +--- +title: Authentication on a published REST service +status: implemented +date: 2026-10-07 +related: + - docs/13-decisions/0010-mdl-canonical-syntax-rules.md + - show-describe-published-rest-services.md +--- + +# Authentication on a published REST service + +> Written for mendixlabs/mxcli#1331. That issue also reported a parser crash on +> a non-string property value; the crash is fixed separately (ako/mxcli#1024) +> and is not part of this proposal. + +## Problem Statement + +`create published rest service` cannot say how a caller authenticates. Studio +Pro's service dialog has **Requires authentication** and, when on, the methods +**Username and password**, **Active session** and **Custom** (with an +authentication microflow). MDL has no spelling for any of them, so: + +- a service mxcli creates is always unauthenticated, and the author has to + finish it in Studio Pro; +- `describe` does not show a service's authentication, so reading a project + through mxcli hides one of its most security-relevant settings; +- the reporter of #1331 fell back to publishing the API as OData, whose + `authentication` clause works, to get custom authentication at all. + +What exists today is a carry, not support: since ako/mxcli#571 the writer copies +the stored `AuthenticationTypes` and `AuthenticationMicroflow` back over its own +constants, so a rewrite does not turn a Studio Pro service's authentication +off. That protects existing services but means MDL can neither set nor read the +setting. + +## BSON Structure + +`Rest$PublishedRestService` (storage name equals qualified name). Two +top-level fields, both present on every service: + +| Field | Shape | Notes | +|---|---|---| +| `AuthenticationTypes` | string list, **marker 1** | values of `Rest$AuthenticationType`: `Basic`, `Session`, `Microflow` (also `Guest`, `None` in the enum) | +| `AuthenticationMicroflow` | by-name reference, stored as a qualified-name string | `""` when none | + +`AuthenticationType` (single) existed 7.11–7.13 and is deleted; +`AuthenticationTypes` since 7.13, `AuthenticationMicroflow` since 7.17 +(`modelsdk/gen/rest/version.go`). Note the list marker differs from the +published OData service, which mxcli has written with 3 and Studio Pro with 1 +(#743) — copying the OData writer is the wrong starting point. + +Measured on ako/TestApp (`Services` module, Studio Pro 11.15.0-rc.4), one +service per dialog state: + +| Service | Dialog | `AuthenticationTypes` | `AuthenticationMicroflow` | `AllowedRoles` | +|---|---|---|---|---| +| `OrdersRestApi` | all three methods | `[1, Basic, Session, Microflow]` | `Services.AuthMicroflow` | `Services.User` | +| `OrdersRestApi_MicroflowSession` | Custom ticked first, then Active session | `[1, Microflow, Session]` | `Services.AuthMicroflow` | `Services.User` | +| `OrdersRestApi_CustomOff` | Custom ticked, microflow chosen, Custom unticked | `[1, Basic, Session]` | `""` | `Services.User` | +| `OrdersRestApi_NoAuth` | Requires authentication = No | `[1]` | `""` | `[1]` (none added) | + +What this settles: + +1. **"Requires authentication = No" is the empty list.** There is no separate + flag, and Studio Pro does not use `None` or `Guest` for it. +2. **Unticking Custom clears `AuthenticationMicroflow`.** No stale reference is + left; mxcli must clear it too whenever `microflow` is not among the methods. +3. **The list keeps the order the methods were ticked.** It is not sorted. + mxcli must write the methods in the order the statement gives them and + `describe` must print the stored order; sorting either side would make an + unchanged `describe` → `exec` rewrite the unit (ADR-0008). +4. **Roles are independent of authentication.** `AllowedRoles` is untouched by + the authentication setting. + +The authentication microflow in the sample takes `$HttpRequest: +System.HttpRequest` and returns `System.User`, empty meaning "not +authenticated". + +## Proposed MDL Syntax + +Authentication is a property of the service (ADR-0010 R9: everything but the +folder and documentation is a property), in the same list as `Path`: + +```mdl +create or modify published rest service Shop.OrdersApi ( + Path: 'rest/orders/v1', + Version: '1.0.0', + ServiceName: 'Orders', + Authentication: (basic, session, microflow Shop.AuthenticateRequest) +) +{ + resource 'orders' { + get '{OrderId}' microflow Shop.GetOrder; + } +}; + +alter published rest service Shop.OrdersApi set (Authentication: (session)); +alter published rest service Shop.OrdersApi set (Authentication: none); +``` + +- `Authentication: none` is "Requires authentication = No". +- `Authentication: ( … )` lists the methods, at least one, in the order + written: `basic` (username and password), `session` (active session), + `microflow Module.Name` (custom). A list-valued property in parentheses is + the shape `Parameters:` and `Headers:` already use. +- **Omitting the property keeps the stored setting** on `create or modify` and + `alter`, as for every other property MDL does not state (`Excluded`, roles). + A new service without it is created unauthenticated, as today. +- `alter … set ( … )` is new for this document type and takes create's keys + (R3): `Path`, `Version`, `ServiceName`, `Authentication`. The existing + `set Key = 'value'` form keeps working for the string properties. +- `describe` prints `Authentication: ( … )` when any method is stored, in the + stored order, and omits it when none is (R12: the creation default is not + printed). A stored value MDL cannot spell — `Guest`, `None`, an unknown + method, or a microflow with no `Microflow` method — is not printed as a + property but noted in a comment, so replaying the output keeps it. + +### Why not the OData clause + +The published OData service spells this as a trailing clause, +`authentication basic, session, microflow M.F`, after the property list. That +predates R9 and cannot be set by its `alter`. Following it here would add a +second non-property spelling and an `alter` gap; following R9 means the two +siblings differ until the OData clause is migrated. The migration is out of +scope here and noted under Open Questions. + +## Validation + +Measured with `mx check` on a fresh 11.14.0 project, one service per variant +(`mxcli new AuthProbe --version 11.14.0`, written by this change): + +| Variant | `mx check` | +|---|---| +| microflow `($HttpRequest: System.HttpRequest) returns System.User` | clean | +| the same with the parameter named `$Req` | clean — matched by type | +| no parameters; `HttpRequest` + `HttpResponse`; `HttpResponse` only | clean | +| returns `Boolean` | **CE0334** "Authentication Microflow … should return a User" | +| a `String` parameter | **CE0336** "The microflow parameter ('Token') is not present in the custom authentication parameter list." | +| custom authentication, app security off | **CE6600** | +| authentication on, no allowed role | **CE0338** "At least one allowed role must be selected …" | +| no authentication, with an allowed role | clean | +| `(microflow M.F, session)` + role, security production | clean | + +So, at `check --references` and `exec` time: + +- **MDL-REST04** (error): the microflow does not exist, does not return + `System.User`, or has a parameter that is not a `System.HttpRequest` or + `System.HttpResponse`. A microflow the same script creates is resolved by name + at check time; `exec` checks its signature once it exists. +- A repeated method (`(basic, basic)`) is a parse-time error; `()` does not parse. +- CE0338 and CE6600 depend on statements that come later (`grant`) or project + state (app security), so they are documented, not predicted. + +## Implementation Plan + +| File | Change | +|------|--------| +| `mdl/grammar/domains/MDLService.g4` | `publishedRestProperty` gains `Key: none` and `Key: ( method, … )`; `publishedRestAuthMethod`; `alterPublishedRestServiceAction` gains `set ( property, … )` | +| `mdl/ast/ast_rest.go` | `PublishedRestAuthentication { Set bool; Methods []string; Microflow string }` on create, and on a new alter action | +| `mdl/visitor/visitor_rest.go`, `visitor_strict_properties.go` | build it; schema shape for `Authentication` | +| `model/types.go` | `PublishedRestService.AuthenticationTypes`, `.AuthenticationMicroflow` | +| `mdl/backend/modelsdk/integration_read.go` | read both fields | +| `mdl/backend/modelsdk/published_rest_write.go` | write both from the model; drop them from `publishedRestServiceUnauthored` | +| `mdl/executor/cmd_published_rest.go` | create / modify / alter: set, or carry the stored value when not stated; `describe` prints the property | +| `mdl/executor/validate.go` | MDL-REST04 at `check --references` | +| `docs-site/`, `mxcli syntax`, skill | document the property | + +Moving the two fields from "carried by the writer" to "carried by the +executor" is the one risky step: every write path must then supply the stored +value itself. There are exactly three (`create`, `create or modify`, `alter`); +`grant`/`revoke` patch `AllowedRoles` only. + +## Version Compatibility + +Both fields exist on every version mxcli supports (10.0+), so no version gate. + +## Test Plan + +- Parser/visitor: each shape, the empty list, a duplicate, an unknown method. +- Backend: write each sample state and compare the encoded fields to the + TestApp table above, marker and order included; read them back. +- Executor (MockBackend): omission carries the stored value on create-or-modify + and alter; `none` clears methods and microflow; dropping `microflow` clears + the microflow; `describe` → `exec` of each TestApp service writes nothing. +- `mx check` (11.14.0): a service per state built by mxcli, plus the + microflow-signature variants, to pin MDL-REST04 to what Mendix accepts + (results under Validation). +- `mdl-examples/doctype-tests/`: a published REST script exercising the property. + +## Open Questions + +1. Migrate the published OData `authentication` clause to an `Authentication:` + property (with the clause as a deprecated alias) so the two siblings agree. + Separate change. +2. ~~Whether a service with roles but no authentication is accepted~~ — it is + (Validation table). The reverse, authentication with no role, is CE0338. diff --git a/mdl-examples/bug-tests/1215-alter-page-insert-tabpage.mdl b/mdl-examples/bug-tests/1215-alter-page-insert-tabpage.mdl new file mode 100644 index 0000000000..76bf8204bb --- /dev/null +++ b/mdl-examples/bug-tests/1215-alter-page-insert-tabpage.mdl @@ -0,0 +1,32 @@ +mdl 1; +-- mendixlabs/mxcli#1215 — ALTER PAGE could not add a tab page to an existing +-- tab container. Both forms failed with +-- +-- failed to insert: failed to build widgets: failed to build widget tpTwo: +-- tabpage must be a direct child of tabcontainer +-- +-- Cause: an INSERT built its widgets one by one, and the builder refuses a lone +-- tab page (it only builds one as a tab container's child). A tab page is also +-- not a widget: it lives in the control's TabPages list, so it takes its own +-- path (InsertTabPages), as list view templates and DataGrid2 columns do. +-- +-- Measured on Mendix 11.14.0: all three inserts below, then `mxcli docker +-- check` → 0 errors, and `describe page` lists tpZero, tpOne, tpTwo, tpThree. +-- With the tab page forced into the sibling tab page's Widgets instead, mx +-- cannot load the project (TabPage has no constructor taking a TabPage). + +create module "ModT"; +create page "ModT"."TabsPage" (Title: 'Tabs', Layout: Atlas_Core.Atlas_Default) { + tabcontainer tabsMain { + tabpage tpOne (Caption: 'One') { + dynamictext txtOne (Content: 'one') + } + } +}; + +-- next to a sibling tab page +alter page "ModT"."TabsPage" { insert after tpOne { tabpage tpTwo (Caption: 'Two') { dynamictext txtTwo (Content: 'two') } } }; +alter page "ModT"."TabsPage" { insert before tpOne { tabpage tpZero (Caption: 'Zero') { dynamictext txtZero (Content: 'zero') } } }; + +-- appended to the tab container +alter page "ModT"."TabsPage" { insert into tabsMain { tabpage tpThree (Caption: 'Three') { dynamictext txtThree (Content: 'three') } } }; diff --git a/mdl-examples/bug-tests/1216-parse-datetime-default-value.mdl b/mdl-examples/bug-tests/1216-parse-datetime-default-value.mdl new file mode 100644 index 0000000000..b38f12e6f7 --- /dev/null +++ b/mdl-examples/bug-tests/1216-parse-datetime-default-value.mdl @@ -0,0 +1,32 @@ +mdl 1; +-- mendixlabs/mxcli#1216: the parse functions take an optional default value, +-- returned when the text does not parse. `check -p` used to reject it: +-- ✗ parseDateTimeUTC() expects 2 argument(s), got 3. [E006] +-- while mx check on 11.14.0 reports 0 errors for every microflow below. +-- (parseBoolean has no such overload: `parseBoolean($s, false)` is CE0117.) + +create module BugTest1216; + +create microflow BugTest1216.SUB_ParseUTC ($Text: String) returns DateTime as $D +begin + declare $D DateTime = parseDateTimeUTC($Text, 'yyyy-MM-dd', empty); + return $D; +end; + +create microflow BugTest1216.SUB_ParseLocal ($Text: String, $Fallback: DateTime) returns DateTime as $D +begin + declare $D DateTime = parseDateTime($Text, 'yyyy-MM-dd', $Fallback); + return $D; +end; + +create microflow BugTest1216.SUB_ParseInteger ($Text: String) returns Integer as $N +begin + declare $N Integer = parseInteger($Text, 0); + return $N; +end; + +create microflow BugTest1216.SUB_ParseDecimal ($Text: String) returns Decimal as $N +begin + declare $N Decimal = parseDecimal($Text, '#.##', 0); + return $N; +end; diff --git a/mdl-examples/bug-tests/1217-commit-clause-in-loop.mdl b/mdl-examples/bug-tests/1217-commit-clause-in-loop.mdl new file mode 100644 index 0000000000..eb7bb966b2 --- /dev/null +++ b/mdl-examples/bug-tests/1217-commit-clause-in-loop.mdl @@ -0,0 +1,66 @@ +mdl 1; +-- upstream mendixlabs/mxcli#1217: "Lint CONV011 (NoCommitInLoop) misses +-- CHANGE … COMMIT (and CREATE … COMMIT) inside a loop; only a separate COMMIT +-- activity is flagged". +-- +-- A create/change activity with Commit = Yes is one database round trip per +-- iteration exactly like a separate commit; Studio Pro's recommender flags both +-- (MXP004). CONV011 and its check-time twin MDL-PERF01 only counted the +-- separate commit activity. +-- +-- Verify: +-- mxcli check mdl-examples/bug-tests/1217-commit-clause-in-loop.mdl +-- expect: MDL-PERF01 on SUB_CloseAll, SUB_CloseAll2 and SUB_LogAll, +-- nothing on SUB_CloseOnce. +-- mxcli exec … -p app.mpr && mxcli lint -p app.mpr -r CONV011 +-- expect: CONV011 on the same three, nothing on SUB_CloseOnce. + +create module CommitClauseLoop; + +create entity CommitClauseLoop.Task ( + "Done": boolean +); + +create entity CommitClauseLoop.LogLine ( + "Text": string +); + +-- The reported shape: commit clause on the change. Was not flagged. +create microflow CommitClauseLoop.SUB_CloseAll($Tasks: list of CommitClauseLoop.Task) +begin + loop $T in $Tasks + begin + change $T ("Done" = true) commit; + end loop; +end; + +-- The shape that was already flagged: separate commit activity. +create microflow CommitClauseLoop.SUB_CloseAll2($Tasks: list of CommitClauseLoop.Task) +begin + loop $T in $Tasks + begin + change $T ("Done" = true); + commit $T; + end loop; +end; + +-- Commit clause on a create, inside a branch inside the loop. +create microflow CommitClauseLoop.SUB_LogAll($Tasks: list of CommitClauseLoop.Task) +begin + loop $T in $Tasks + begin + if $T/Done then + $L = create CommitClauseLoop.LogLine ("Text" = 'done') commit; + end if; + end loop; +end; + +-- CONTROL: change in the loop, commit the list once after it. +create microflow CommitClauseLoop.SUB_CloseOnce($Tasks: list of CommitClauseLoop.Task) +begin + loop $T in $Tasks + begin + change $T ("Done" = true); + end loop; + commit $Tasks; +end; diff --git a/mdl-examples/bug-tests/1324-own-data-container-name-outside-actions.fail.mdl b/mdl-examples/bug-tests/1324-own-data-container-name-outside-actions.fail.mdl new file mode 100644 index 0000000000..f09cead85b --- /dev/null +++ b/mdl-examples/bug-tests/1324-own-data-container-name-outside-actions.fail.mdl @@ -0,0 +1,30 @@ +-- mendixlabs/mxcli#1324 follow-up: a data container's own name outside actions +-- +-- NEGATIVE TEST (.fail.mdl) — EXPECTED to fail `mxcli check`. +-- `make check-mdl` inverts the exit code: an unexpected pass is a regression of +-- MDL-BUTTON02's non-action slots. +-- +-- Symptom: `check` and `exec` were clean, then `mx check` (Mendix 11.14.0) +-- reported, for widgets placed directly inside data view dvP: +-- [CE0117] "Error(s) in expression." at Container 'cVisOwn' +-- and the same for a nested data view's microflow data-source argument, +-- `Editable:` and `DynamicClasses:`, plus CE0161 "Error(s) in XPath +-- constraint." for a nested list's `where`. These slots are evaluated in the +-- enclosing context, where dvP's object is $currentObject; `$dvP` is a +-- variable only one data container deeper (cVisOuter builds clean). + +create module G52; +create persistent entity G52.Gate (Name: String(50)); + +create page G52.VisibilityPage (Title: 'Gate', Layout: Atlas_Core.PopupLayout, Params: ( $Gate: G52.Gate )) { + dataview dvP (DataSource: $Gate) { + container cVisOwn (Visible: $dvP/Name != '') { + dynamictext dtA (Content: 'a') + } + dataview dvMid (DataSource: $Gate) { + container cVisOuter (Visible: $dvP/Name != '') { + dynamictext dtB (Content: 'b') + } + } + } +}; diff --git a/mdl-examples/bug-tests/1328-excluded-activity.mdl b/mdl-examples/bug-tests/1328-excluded-activity.mdl new file mode 100644 index 0000000000..ac39e8569d --- /dev/null +++ b/mdl-examples/bug-tests/1328-excluded-activity.mdl @@ -0,0 +1,35 @@ +mdl 1; +-- ============================================================================ +-- mendixlabs/mxcli#1328 — "@excluded on microflow activities does not seem to +-- work anymore" +-- ============================================================================ +-- +-- `@excluded declare $Variable Boolean = false;` parsed and `exec` reported +-- success, but the activity was written enabled (Disabled = false), so +-- DESCRIBE printed no @excluded and the round trip lost it. +-- +-- The flow builder merges each statement's annotations into a pending set by +-- hand, field by field, and never copied Excluded. Guard: +-- mdl/executor/issue1328_excluded_activity_test.go. +-- +-- After exec, DESCRIBE of each microflow must print @excluded above the +-- marked activity, and only that one. +-- ============================================================================ + +CREATE MODULE BugExcluded1328; + +CREATE OR MODIFY MICROFLOW BugExcluded1328.ACT_disabled_activity () +BEGIN + @excluded + DECLARE $Variable Boolean = false; + RETURN; +END; + +CREATE OR MODIFY MICROFLOW BugExcluded1328.ACT_disabled_log () +BEGIN + LOG INFO NODE 'BugExcluded1328' 'enabled'; + @excluded + @caption 'Disabled log' + LOG INFO NODE 'BugExcluded1328' 'disabled'; + RETURN; +END; diff --git a/mdl-examples/bug-tests/1331-published-rest-non-string-property.fail.mdl b/mdl-examples/bug-tests/1331-published-rest-non-string-property.fail.mdl new file mode 100644 index 0000000000..2d50d5e1a5 --- /dev/null +++ b/mdl-examples/bug-tests/1331-published-rest-non-string-property.fail.mdl @@ -0,0 +1,28 @@ +-- mendixlabs/mxcli#1331: a published REST service property whose value is not +-- a string literal +-- +-- NEGATIVE TEST (.fail.mdl) — EXPECTED to fail `mxcli check` with a syntax error. +-- +-- Symptom: `check` and `exec` crashed instead of reporting the error: +-- panic: runtime error: invalid memory address or nil pointer dereference +-- ... visitor.unquoteStringLit ... visitor_rest.go +-- Build walks a failed parse on purpose, so the property's rule context arrived +-- without its STRING_LITERAL child. `Authentication: Basic` and +-- `Folder: microflow M.F` took the same path. +-- +-- Fix: unquoteStringLit reads a missing node as "", so the author sees +-- line 22:18 mismatched input 'microflow' expecting STRING_LITERAL +-- A panic also exits non-zero, so this file cannot tell the two apart; the +-- guard is TestCreatePublishedRestService_NonStringPropertyValue_NoPanic. + +create or modify published rest service MyFirstModule.TestApi ( + Path: 'rest/test/v1', + Version: '1.0.0', + ServiceName: 'Test API', + Authentication: microflow MyFirstModule.AuthMf +) +{ + resource 'items' { + get '' microflow MyFirstModule.GetItems; + } +}; diff --git a/mdl-examples/bug-tests/check-unknown-declared-entity-type.fail.mdl b/mdl-examples/bug-tests/check-unknown-declared-entity-type.fail.mdl new file mode 100644 index 0000000000..17375c5ceb --- /dev/null +++ b/mdl-examples/bug-tests/check-unknown-declared-entity-type.fail.mdl @@ -0,0 +1,34 @@ +-- check --references passed a declared type naming an entity that does not exist. +-- +-- Reported as `create association M.X_Y from System.Nope to M.Y` passing +-- `mxcli check script.mdl -p app.mpr --references`. On current main that shape +-- is refused (association endpoints are resolved since ako/mxcli#555), but the +-- same unknown name in a flow's or page's SIGNATURE was not resolved at all: +-- +-- mxcli check this-file -p app.mpr --references +-- before: "✓ All references valid" / "Check passed!" +-- after: one reference error per statement below, naming the entity +-- +-- Measured on Evora Factory Management (Mendix 10.24.15). exec refuses the +-- user-module shapes and every page/snippet parameter after the statements +-- before it are written; a System entity in a flow signature it writes by +-- name, leaving a reference nothing resolves. +-- +-- Every statement below must be refused. The control script beside it, +-- check-unknown-declared-entity-type.mdl, must pass. + +mdl 1; + +create module BugDeclType; +create persistent entity BugDeclType.Thing (Name: String(100)); + +-- the reported shape (already refused; kept as the regression) +create association BugDeclType.Nope_Thing from System.Nope to BugDeclType.Thing; + +create microflow BugDeclType.MF_SystemParam ($p: System.Nope) begin end; +create microflow BugDeclType.MF_UserParam ($p: BugDeclType.Nope) begin end; +create microflow BugDeclType.MF_ListParam ($p: list of System.Nope) begin end; +create microflow BugDeclType.MF_Return () returns System.Nope begin return empty; end; +create nanoflow BugDeclType.NF_Param ($p: System.Nope) begin end; +create page BugDeclType.P_Param (params: ($x: System.Nope), title: 'x', layout: Atlas_Core.Atlas_Default) { }; +create snippet BugDeclType.S_Param (params: ($x: BugDeclType.Nope)) { }; diff --git a/mdl-examples/bug-tests/check-unknown-declared-entity-type.mdl b/mdl-examples/bug-tests/check-unknown-declared-entity-type.mdl new file mode 100644 index 0000000000..38102e6d50 --- /dev/null +++ b/mdl-examples/bug-tests/check-unknown-declared-entity-type.mdl @@ -0,0 +1,24 @@ +-- Control for check-unknown-declared-entity-type.fail.mdl: real System entities +-- and enumerations in declared types must keep passing check --references. +-- Requires a project with the Atlas_Core module. + +mdl 1; + +create module BugDeclTypeOk; +create persistent entity BugDeclTypeOk.Thing (Name: String(100)); + +create association BugDeclTypeOk.Thing_User from BugDeclTypeOk.Thing to System.User; + +create microflow BugDeclTypeOk.MF_Ok ( + $u: System.User, + $f: list of System.FileDocument, + $d: System.DeviceType, + $t: BugDeclTypeOk.Thing +) returns System.Image +begin + return empty; +end; + +create nanoflow BugDeclTypeOk.NF_Ok ($s: System.Session) begin end; + +create page BugDeclTypeOk.P_Ok (params: ($x: System.User), title: 'x', layout: Atlas_Core.Atlas_Default) { }; diff --git a/mdl-examples/bug-tests/current-datetime-function.fail.mdl b/mdl-examples/bug-tests/current-datetime-function.fail.mdl new file mode 100644 index 0000000000..0961379acf --- /dev/null +++ b/mdl-examples/bug-tests/current-datetime-function.fail.mdl @@ -0,0 +1,16 @@ +mdl 1; +-- currentDateTime() is not a Mendix expression function. mxcli listed it as a +-- built-in, so `check` and `exec` passed this microflow and mx check on 11.14.0 +-- then failed it (and the nanoflow form alike) with: +-- [error] [CE0117] "Error(s) in expression." at Create variable activity +-- 'Create Date and time variable' +-- MDL044 must refuse it and point at the [%CurrentDateTime%] token. +-- Positive control: current-datetime-token.mdl. + +create module BugCurrentDateTime; + +create microflow BugCurrentDateTime.MF_Now () returns DateTime as $D +begin + declare $D DateTime = currentDateTime(); + return $D; +end; diff --git a/mdl-examples/bug-tests/current-datetime-token.mdl b/mdl-examples/bug-tests/current-datetime-token.mdl new file mode 100644 index 0000000000..8d1570fdb5 --- /dev/null +++ b/mdl-examples/bug-tests/current-datetime-token.mdl @@ -0,0 +1,19 @@ +mdl 1; +-- The current time is the [%CurrentDateTime%] token. This is the positive +-- control for current-datetime-function.fail.mdl: the same microflow and +-- nanoflow with the token must pass, and on Mendix 11.14.0 both build at +-- 0 errors. + +create module BugCurrentDateTimeOK; + +create microflow BugCurrentDateTimeOK.MF_Now () returns DateTime as $D +begin + declare $D DateTime = addDays([%CurrentDateTime%], 1); + return $D; +end; + +create nanoflow BugCurrentDateTimeOK.NF_Now () returns DateTime as $D +begin + declare $D DateTime = [%CurrentDateTime%]; + return $D; +end; diff --git a/mdl-examples/bug-tests/structure-page-data-widgets.mdl b/mdl-examples/bug-tests/structure-page-data-widgets.mdl new file mode 100644 index 0000000000..c41e32885b --- /dev/null +++ b/mdl-examples/bug-tests/structure-page-data-widgets.mdl @@ -0,0 +1,66 @@ +mdl 1; +-- Bug test: `describe structure depth 2|3` (CLI: `mxcli structure -d 3`) is +-- meant to annotate each page with its data widgets - +-- Page M.P [DataView, DataGrid] +-- and never did, on any project: every page printed bare. +-- +-- Cause: the annotation query filtered on `ParentWidget = ''`, a column the +-- catalog's widgets table has never had (the tree position is ParentWidgetId), +-- and the query error was discarded. It was that way from the initial commit. +-- +-- The annotation lists a page's OUTERMOST data widgets - those with no data +-- widget above them. A data grid inside a layout grid counts (a layout grid is +-- not a data context); a list view inside a data view does not. Built-in +-- widgets only, so the script runs on a project without widget packages. +-- +-- The widgets table is filled by a full catalog build only, so refresh first. +-- +-- Expected after the fix, for page StructPageWidgets.Customer_Orders: +-- Page StructPageWidgets.Customer_Orders [DataView, ListView] +-- once: `allOrders` is listed, `customerOrders` is not (it is nested inside +-- the data view). Both sit inside the layout grid, so filtering on the page +-- root would have listed neither. + +create module StructPageWidgets; + +create persistent entity StructPageWidgets.Customer ( + Name: String(200) +); + +create persistent entity StructPageWidgets.CustomerOrder ( + Number: String(20) +); + +create page StructPageWidgets.Customer_Orders +( + params: ( + $Customer: StructPageWidgets.Customer + ), + title: 'Customer orders', + layout: Atlas_Core.Atlas_Default +) +{ + layoutgrid mainGrid { + row { + column (desktopwidth: autofill) { + listview allOrders (datasource: database StructPageWidgets.CustomerOrder) { + dynamictext allOrderNumber (content: '{1}', contentparams: ({1} = Number)) + } + } + } + row { + column (desktopwidth: autofill) { + dataview customerView (datasource: $Customer) { + textbox customerName (label: 'Name', attribute: Name) + listview customerOrders (datasource: database StructPageWidgets.CustomerOrder) { + dynamictext orderNumber (content: '{1}', contentparams: ({1} = Number)) + } + } + } + } + } +}; + +refresh catalog full; + +describe structure depth 3 in StructPageWidgets; diff --git a/mdl-examples/doctype-tests/22-published-rest-service-examples.mdl b/mdl-examples/doctype-tests/22-published-rest-service-examples.mdl index 5f90141b4c..801b5219b2 100644 --- a/mdl-examples/doctype-tests/22-published-rest-service-examples.mdl +++ b/mdl-examples/doctype-tests/22-published-rest-service-examples.mdl @@ -156,6 +156,42 @@ describe published rest service PrsTest.OrderAPI; alter published rest service PrsTest.OrderAPI drop resource 'shipments'; +-- ############################################################################ +-- PART 7b: AUTHENTICATION (mendixlabs/mxcli#1331) +-- ############################################################################ +-- +-- The methods are stored in the order written. `none` is "Requires +-- authentication: No"; leaving the property out keeps the stored setting. +-- With authentication on, at least one role must be granted (CE0338). +-- `microflow Module.Name` (custom) also needs app security on (CE6600), which +-- this script does not switch on. + +create published rest service PrsTest.SecureAPI ( + Path: 'rest/secure/v1', + Version: '1.0.0', + ServiceName: 'Secure API', + Authentication: (session, basic) +) +{ + resource 'orders' { + get '' microflow PrsTest.PRS_GetAllOrders; + } +}; + +grant access on published rest service PrsTest.SecureAPI to PrsTest.user; + +describe published rest service PrsTest.SecureAPI; + +alter published rest service PrsTest.SecureAPI set (Version: '1.1.0'); + +alter published rest service PrsTest.SecureAPI set (Authentication: (basic)); + +describe published rest service PrsTest.SecureAPI; + +alter published rest service PrsTest.SecureAPI set (Authentication: none); + +drop published rest service PrsTest.SecureAPI; + -- ############################################################################ -- PART 8: DROP -- ############################################################################ diff --git a/mdl/ast/ast_rest.go b/mdl/ast/ast_rest.go index 225d8e6b43..d924a43810 100644 --- a/mdl/ast/ast_rest.go +++ b/mdl/ast/ast_rest.go @@ -107,16 +107,29 @@ func (s *DescribeContractFromOpenAPIStmt) isStatement() {} // // CREATE PUBLISHED REST SERVICE Module.Name (Path: '...', Version: '...') { RESOURCE ... }; type CreatePublishedRestServiceStmt struct { - CreateGuard // `create … if not exists` (ako/mxcli#731) - Name QualifiedName - Path string - Version string - ServiceName string - Folder string + CreateGuard // `create … if not exists` (ako/mxcli#731) + Name QualifiedName + Path string + Version string + ServiceName string + Folder string + // Authentication is nil when the statement does not state it, which keeps + // the stored setting on create or modify. + Authentication *PublishedRestAuthentication Resources []*PublishedRestResourceDef CreateOrModify bool } +// PublishedRestAuthentication is `Authentication: none | ( method, … )`. +// Methods holds the stored spellings ("Basic", "Session", "Microflow") in the +// order written, which is the order Studio Pro stores them; empty is `none`. +// Microflow is the custom-authentication microflow, set exactly when Methods +// holds "Microflow". +type PublishedRestAuthentication struct { + Methods []string + Microflow string +} + func (s *CreatePublishedRestServiceStmt) isStatement() {} type PublishedRestResourceDef struct { @@ -160,9 +173,11 @@ type PublishedRestAlterAction interface { isPublishedRestAlterAction() } -// PublishedRestSetAction represents: SET key = 'value' [, ...] +// PublishedRestSetAction represents: SET key = 'value' [, ...] or +// SET ( Key: value, ... ). Authentication is nil unless the list states it. type PublishedRestSetAction struct { - Changes map[string]string + Changes map[string]string + Authentication *PublishedRestAuthentication } func (a *PublishedRestSetAction) isPublishedRestAlterAction() {} diff --git a/mdl/backend/mcp/page_mutator.go b/mdl/backend/mcp/page_mutator.go index a140156095..0f70773c89 100644 --- a/mdl/backend/mcp/page_mutator.go +++ b/mdl/backend/mcp/page_mutator.go @@ -502,6 +502,10 @@ func (m *mcpPageMutator) InsertListViewTemplates(listViewRef string, _ []*pages. return fmt.Errorf("adding specialization templates to %s is not yet supported by the MCP backend", listViewRef) } +func (m *mcpPageMutator) InsertTabPages(targetRef string, _ backend.InsertPosition, _ []*pages.TabPage) error { + return fmt.Errorf("adding tab pages at %s is not yet supported by the MCP backend", targetRef) +} + func (m *mcpPageMutator) DropListViewTemplate(listViewRef, specialization string) error { return fmt.Errorf("dropping the %s template from %s is not yet supported by the MCP backend", specialization, listViewRef) diff --git a/mdl/backend/mock/mock_page_mutator.go b/mdl/backend/mock/mock_page_mutator.go index df1b79e41c..93ea289eea 100644 --- a/mdl/backend/mock/mock_page_mutator.go +++ b/mdl/backend/mock/mock_page_mutator.go @@ -32,6 +32,7 @@ type MockPageMutator struct { ReplaceWidgetFunc func(widgetRef string, columnRef string, widgets []pages.Widget) error InsertColumnsFunc func(gridRef, afterColumnRef string, position backend.InsertPosition, columns []*backend.DataGridColumnSpec) error InsertListViewTemplatesFunc func(listViewRef string, templates []*pages.ListViewTemplate) error + InsertTabPagesFunc func(targetRef string, position backend.InsertPosition, tabPages []*pages.TabPage) error DropListViewTemplateFunc func(listViewRef, specialization string) error ReplaceColumnFunc func(gridRef, columnRef string, columns []*backend.DataGridColumnSpec) error FindWidgetFunc func(name string) bool @@ -146,6 +147,13 @@ func (m *MockPageMutator) InsertListViewTemplates(listViewRef string, templates return fmt.Errorf("MockBackend.InsertListViewTemplates not configured") } +func (m *MockPageMutator) InsertTabPages(targetRef string, position backend.InsertPosition, tabPages []*pages.TabPage) error { + if m.InsertTabPagesFunc != nil { + return m.InsertTabPagesFunc(targetRef, position, tabPages) + } + return fmt.Errorf("MockBackend.InsertTabPages not configured") +} + func (m *MockPageMutator) DropListViewTemplate(listViewRef, specialization string) error { if m.DropListViewTemplateFunc != nil { return m.DropListViewTemplateFunc(listViewRef, specialization) diff --git a/mdl/backend/modelsdk/integration_read.go b/mdl/backend/modelsdk/integration_read.go index 49d7591742..ebc66c37b7 100644 --- a/mdl/backend/modelsdk/integration_read.go +++ b/mdl/backend/modelsdk/integration_read.go @@ -411,6 +411,9 @@ func (b *Backend) ListPublishedRestServices() ([]*model.PublishedRestService, er ServiceName: g.ServiceName(), Excluded: g.Excluded(), AllowedRoles: append([]string(nil), g.AllowedRolesQualifiedNames()...), + + AuthenticationTypes: append([]string(nil), g.AuthenticationTypesItems()...), + AuthenticationMicroflow: g.AuthenticationMicroflowQualifiedName(), } svc.ID = model.ID(g.ID()) svc.TypeName = "Rest$PublishedRestService" diff --git a/mdl/backend/modelsdk/page_mutator_tabpages_test.go b/mdl/backend/modelsdk/page_mutator_tabpages_test.go new file mode 100644 index 0000000000..7e814f03bb --- /dev/null +++ b/mdl/backend/modelsdk/page_mutator_tabpages_test.go @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "testing" + + "go.mongodb.org/mongo-driver/bson" + + "github.com/mendixlabs/mxcli/mdl/backend/bsonnav" + "github.com/mendixlabs/mxcli/mdl/backend/pagemutator" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// mendixlabs/mxcli#1215: InsertTabPages serializes the new pages through the +// codec by wrapping them in a tab container. Through the real serializer, the +// stored page must be a Forms$TabPage with its name, caption and children, and +// keep the $ID the builder gave it (registered for duplicate-name checks). +func TestInsertTabPages_CodecSerializer(t *testing.T) { + existing := bson.D{ + {Key: "$ID", Value: "tp1"}, + {Key: "$Type", Value: "Forms$TabPage"}, + {Key: "Name", Value: "tpOne"}, + {Key: "Widgets", Value: bson.A{int32(2)}}, + } + page := bson.D{ + {Key: "$Type", Value: "Forms$Page"}, + {Key: "FormCall", Value: bson.D{ + {Key: "$Type", Value: "Forms$LayoutCall"}, + {Key: "Arguments", Value: bson.A{int32(2), bson.D{ + {Key: "$Type", Value: "Forms$FormCallArgument"}, + {Key: "Widgets", Value: bson.A{int32(2), bson.D{ + {Key: "$Type", Value: "Forms$TabControl"}, + {Key: "Name", Value: "tabsMain"}, + {Key: "DefaultPagePointer", Value: "tp1"}, + {Key: "TabPages", Value: bson.A{int32(3), existing}}, + }}}, + }}}, + }}, + } + m := pagemutator.New(page, model.ID("page-1"), codecPageDeps{}) + + id := types.GenerateID() + tp := &pages.TabPage{ + BaseElement: model.BaseElement{ID: model.ID(id), TypeName: "Forms$TabPage"}, + Name: "tpTwo", + Caption: &model.Text{Translations: map[string]string{"en_US": "Two"}}, + Widgets: []pages.Widget{&pages.DynamicText{BaseWidget: pages.BaseWidget{ + BaseElement: model.BaseElement{ID: model.ID(types.GenerateID()), TypeName: "Forms$DynamicText"}, + Name: "txtTwo", + }}}, + } + if err := m.InsertTabPages("tpOne", "after", []*pages.TabPage{tp}); err != nil { + t.Fatalf("insert after tpOne: %v", err) + } + + ctl := bsonnav.DGetArrayElements(bsonnav.DGet(bsonnav.DGetArrayElements(bsonnav.DGet( + bsonnav.DGetDoc(page, "FormCall"), "Arguments"))[0].(bson.D), "Widgets"))[0].(bson.D) + tabs := bsonnav.DGetArrayElements(bsonnav.DGet(ctl, "TabPages")) + if len(tabs) != 2 { + t.Fatalf("TabPages has %d entries, want 2", len(tabs)) + } + added := tabs[1].(bson.D) + if got := bsonnav.DGetString(added, "$Type"); got != "Forms$TabPage" { + t.Errorf("$Type = %q, want Forms$TabPage", got) + } + if got := bsonnav.DGetString(added, "Name"); got != "tpTwo" { + t.Errorf("Name = %q, want tpTwo", got) + } + if bsonnav.DGetDoc(added, "Caption") == nil { + t.Error("Caption missing") + } + kids := bsonnav.DGetArrayElements(bsonnav.DGet(added, "Widgets")) + if len(kids) != 1 || bsonnav.DGetString(kids[0].(bson.D), "Name") != "txtTwo" { + t.Errorf("Widgets = %v, want txtTwo", kids) + } + if bsonnav.DGetString(ctl, "DefaultPagePointer") != "tp1" { + t.Error("DefaultPagePointer moved off the existing default page") + } +} diff --git a/mdl/backend/modelsdk/published_rest_auth_test.go b/mdl/backend/modelsdk/published_rest_auth_test.go new file mode 100644 index 0000000000..86fc844af7 --- /dev/null +++ b/mdl/backend/modelsdk/published_rest_auth_test.go @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "reflect" + "testing" + + "go.mongodb.org/mongo-driver/bson" + + "github.com/mendixlabs/mxcli/model" +) + +// storedAuth reads the two authentication fields as stored. +func storedAuth(t *testing.T, b *Backend, id model.ID) (bson.A, string) { + t.Helper() + raw, err := b.reader.GetRawUnitBytes(string(id)) + if err != nil { + t.Fatalf("raw unit: %v", err) + } + var d struct { + AuthenticationTypes bson.A `bson:"AuthenticationTypes"` + AuthenticationMicroflow string `bson:"AuthenticationMicroflow"` + } + if err := bson.Unmarshal(raw, &d); err != nil { + t.Fatalf("unmarshal: %v", err) + } + return d.AuthenticationTypes, d.AuthenticationMicroflow +} + +// TestPublishedRestService_WritesAuthentication pins the writer to what Studio +// Pro stores for each dialog state, measured on ako/TestApp's Services module +// (mendixlabs/mxcli#1331): a marker-1 list in the order the methods were +// ticked, the microflow by qualified name, and the empty list with an empty +// microflow for "no authentication". Each update states the setting; the +// carry of an unstated one is the executor's (cmd_published_rest.go). +func TestPublishedRestService_WritesAuthentication(t *testing.T) { + proj := copyFixture(t) + b := New() + if err := b.Connect(proj); err != nil { + t.Fatalf("connect: %v", err) + } + t.Cleanup(func() { _ = b.Disconnect() }) + mod, err := b.GetModuleByName("MyFirstModule") + if err != nil || mod == nil { + t.Fatalf("GetModuleByName: %v", err) + } + svc := &model.PublishedRestService{ + ContainerID: mod.ID, Name: "ZzAuth", Path: "rest/zza/v1", + // TestApp's OrdersRestApi_MicroflowSession: Custom ticked first. + AuthenticationTypes: []string{"Microflow", "Session"}, + AuthenticationMicroflow: "MyFirstModule.ACT_Auth", + } + if err := b.CreatePublishedRestService(svc); err != nil { + t.Fatalf("create: %v", err) + } + + steps := []struct { + name string + types []string + mf string + wantTypes bson.A + }{ + {"created", svc.AuthenticationTypes, svc.AuthenticationMicroflow, bson.A{int32(1), "Microflow", "Session"}}, + // OrdersRestApi_CustomOff: Custom unticked, microflow cleared. + {"custom off", []string{"Basic", "Session"}, "", bson.A{int32(1), "Basic", "Session"}}, + // OrdersRestApi_NoAuth. + {"none", nil, "", bson.A{int32(1)}}, + } + for i, st := range steps { + if i > 0 { + svc.AuthenticationTypes, svc.AuthenticationMicroflow = st.types, st.mf + if err := b.UpdatePublishedRestService(svc); err != nil { + t.Fatalf("%s: update: %v", st.name, err) + } + } + types, mf := storedAuth(t, b, svc.ID) + if !reflect.DeepEqual(types, st.wantTypes) || mf != st.mf { + t.Errorf("%s: stored %v / %q, want %v / %q", st.name, types, mf, st.wantTypes, st.mf) + } + all, err := b.ListPublishedRestServices() + if err != nil { + t.Fatalf("list: %v", err) + } + for _, s := range all { + if s.Name == "ZzAuth" && (!reflect.DeepEqual(s.AuthenticationTypes, st.types) && len(s.AuthenticationTypes)+len(st.types) > 0 || s.AuthenticationMicroflow != st.mf) { + t.Errorf("%s: read back %v / %q, want %v / %q", st.name, s.AuthenticationTypes, s.AuthenticationMicroflow, st.types, st.mf) + } + } + } +} diff --git a/mdl/backend/modelsdk/published_rest_write.go b/mdl/backend/modelsdk/published_rest_write.go index 919db33b7e..8d79f16d29 100644 --- a/mdl/backend/modelsdk/published_rest_write.go +++ b/mdl/backend/modelsdk/published_rest_write.go @@ -131,12 +131,14 @@ func (b *Backend) UpdatePublishedRestServiceRoles(unitID model.ID, roles []strin // publishedRestServiceUnauthored are the Rest$PublishedRestService keys a // create or modify / alter cannot state: the writer emits a constant for each, -// so a rewrite carries the stored value instead. Without the carry, executing -// the describe output of a Studio Pro service turned its Basic and Session -// authentication off (ako/mxcli#571). +// so a rewrite carries the stored value instead. +// +// AuthenticationTypes and AuthenticationMicroflow were here until MDL could +// state them (mendixlabs/mxcli#1331); before that, executing the describe +// output of a Studio Pro service turned its Basic and Session authentication +// off (ako/mxcli#571). They are written from the model now, and an unstated +// setting is carried by the executor, which reads it with the service. var publishedRestServiceUnauthored = []string{ - "AuthenticationMicroflow", - "AuthenticationTypes", "CorsConfiguration", "Documentation", "Parameters", @@ -155,9 +157,12 @@ func publishedRestServiceToGen(svc *model.PublishedRestService) element.Element if len(svc.AllowedRoles) > 0 { addByNameRefList(g, "AllowedRoles", "Security$ModuleRole", svc.AllowedRoles) } - // AllowedRoles (empty), AuthenticationTypes, Parameters, CorsConfiguration: - // emitted via the registered TypeDefaults. - addStr(g, "AuthenticationMicroflow", "") + // AllowedRoles (empty), Parameters, CorsConfiguration: emitted via the + // registered TypeDefaults. AuthenticationTypes is a marker-1 string list in + // the order given, which is the order Studio Pro stores the ticked methods + // in; empty is "Requires authentication = No" (ako/TestApp, #1331). + addStrList(g, "AuthenticationTypes", svc.AuthenticationTypes) + addStr(g, "AuthenticationMicroflow", svc.AuthenticationMicroflow) resources := make([]element.Element, 0, len(svc.Resources)) for _, res := range svc.Resources { diff --git a/mdl/backend/mutation.go b/mdl/backend/mutation.go index bfb597fe53..02930d2e65 100644 --- a/mdl/backend/mutation.go +++ b/mdl/backend/mutation.go @@ -144,6 +144,12 @@ type PageMutator interface { // list, which Studio Pro cannot open. InsertListViewTemplates(listViewRef string, templates []*pages.ListViewTemplate) error + // InsertTabPages adds tab pages to a tab container: INTO the container + // appends, BEFORE/AFTER a tab page places them next to that sibling. A tab + // page lives in the container's TabPages list and is not a widget, so it + // cannot go through InsertWidget (#1215). + InsertTabPages(targetRef string, position InsertPosition, tabPages []*pages.TabPage) error + // DropListViewTemplate removes the template rendering the given // specialization from a List View. A template has no name, so it is addressed // by entity. Returns an error naming the templates that ARE present when the diff --git a/mdl/backend/pagemutator/mutator.go b/mdl/backend/pagemutator/mutator.go index 4aef9b275d..1bce62f1f2 100644 --- a/mdl/backend/pagemutator/mutator.go +++ b/mdl/backend/pagemutator/mutator.go @@ -424,20 +424,22 @@ func refuseWidgetsAtColumnTarget(gridRef, columnRef string) error { // lives in the control's TabPages list rather than a widget list. const tabPageType = "Forms$TabPage" -// refuseTabPageSiblingEdit refuses an operation that would edit the TabPages -// list itself: INSERT BEFORE/AFTER and REPLACE would put ordinary widgets into a -// list that holds only tab pages, and DROP can remove the page the control's -// DefaultPagePointer names, leaving a dangling pointer. What a tab page does -// support — SET Caption / Visible / Name and INSERT INTO it — goes through the -// ordinary widget paths. +// refuseTabPageSiblingEdit refuses an operation that would put ordinary widgets +// into, or remove entries from, the TabPages list: INSERT BEFORE/AFTER and +// REPLACE would put widgets into a list that holds only tab pages, and DROP can +// remove the page the control's DefaultPagePointer names, leaving a dangling +// pointer. What a tab page does support — SET Caption / Visible / Name, INSERT +// INTO it — goes through the ordinary widget paths; adding a tab page next to +// it goes through InsertTabPages (#1215). func refuseTabPageSiblingEdit(result *bsonWidgetResult, name, op string) error { if result == nil || bsonnav.DGetString(result.widget, "$Type") != tabPageType { return nil } - return fmt.Errorf("%q is a tab page: %s would edit the tab container's list of pages, "+ - "which ALTER PAGE does not do. Use `insert into %s { … }` to add widgets to it, "+ + return fmt.Errorf("%q is a tab page: %s would put widgets into, or remove a page from, the tab "+ + "container's list of pages, which ALTER PAGE does not do. Use `insert into %s { … }` to add "+ + "widgets to it, `insert after %s { tabpage … }` to add a tab page next to it, "+ "`set Visible = false on %s` to hide it, or `describe page` and `create or modify page` "+ - "to add, remove or reorder tab pages", name, op, name, name) + "to remove or reorder tab pages", name, op, name, name, name) } func (m *Mutator) InsertWidget(widgetRef string, columnRef string, position backend.InsertPosition, widgets []pages.Widget) error { diff --git a/mdl/backend/pagemutator/tabpages.go b/mdl/backend/pagemutator/tabpages.go new file mode 100644 index 0000000000..49dc7a894e --- /dev/null +++ b/mdl/backend/pagemutator/tabpages.go @@ -0,0 +1,129 @@ +// SPDX-License-Identifier: Apache-2.0 + +package pagemutator + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/backend/bsonnav" + "github.com/mendixlabs/mxcli/sdk/pages" + "go.mongodb.org/mongo-driver/bson" +) + +// isTabControl reports whether a stored widget is a tab container. +func isTabControl(widget bson.D) bool { + switch widgetTypeName(widget) { + case "Forms$TabControl", "Pages$TabControl": + return true + } + return false +} + +// InsertTabPages adds tab pages to an existing tab container. +// +// A tab page lives in the control's TabPages list, not in a Widgets list, and +// is not a widget — the same reason list view templates and DataGrid2 columns +// have their own paths. INSERT INTO appends; INSERT BEFORE/AFTER +// places the new pages next to that sibling. Anything else would put +// a tab page where only widgets belong, or a widget where only tab pages do. +// +// The pages are serialized by wrapping them in a throwaway tab container and +// taking its TabPages back out, so they get exactly the shape CREATE PAGE +// writes. The wrapper's DefaultPagePointer is discarded: the control keeps the +// default page it already had, and only an empty control gains one. +func (m *Mutator) InsertTabPages(targetRef string, position backend.InsertPosition, tabPages []*pages.TabPage) error { + result := m.widgetFinder(m.rawData, targetRef) + if result == nil { + return m.widgetNotFoundError(targetRef) + } + into := strings.EqualFold(string(position), "into") + isPage := widgetTypeName(result.widget) == tabPageType + switch { + case into && !isTabControl(result.widget): + hint := "" + if isPage { + hint = fmt.Sprintf(" — to add a tab page next to %q, use `insert after %s { tabpage … }`", targetRef, targetRef) + } + return fmt.Errorf("cannot insert a tab page into %q (%s): a tab page can only be added to a tab container%s", + targetRef, widgetTypeName(result.widget), hint) + case !into && !isPage: + return fmt.Errorf("cannot insert a tab page %s %q (%s): a tab page's siblings are tab pages — "+ + "use `insert %s { tabpage … }` or `insert into { tabpage … }`", + strings.ToLower(string(position)), targetRef, widgetTypeName(result.widget), strings.ToLower(string(position))) + } + + newDocs, err := m.serializeTabPages(tabPages) + if err != nil { + return err + } + + if !into { + // result.parentArr is the control's TabPages list and result.index the + // sibling's slot in it, marker included. + insertIdx := result.index + if strings.EqualFold(string(position), "after") { + insertIdx++ + } + out := make([]any, 0, len(result.parentArr)+len(newDocs)) + out = append(out, result.parentArr[:insertIdx]...) + out = append(out, newDocs...) + out = append(out, result.parentArr[insertIdx:]...) + bsonnav.DSetArray(result.parentDoc, result.parentKey, out) + return nil + } + + control := result.widget + stored := bsonnav.ToBsonA(bsonnav.DGet(control, "TabPages")) + var out bson.A + switch { + case len(stored) > 0 && isListMarker(stored[0]): + out = append(out, stored...) + case len(stored) == 0: + out = append(out, int32(3)) // TabPages' list marker as Studio Pro writes it + default: + out = append(out, stored...) + } + hadPages := len(out) > 1 || (len(out) == 1 && !isListMarker(out[0])) + out = append(out, newDocs...) + + if !bsonnav.DSet(control, "TabPages", out) { + control = append(control, bson.E{Key: "TabPages", Value: out}) + } + // An empty control has no default page; the first tab page is Studio Pro's + // default, so give it one rather than leave the pointer null. + if !hadPages { + firstID := bsonnav.DGet(newDocs[0].(bson.D), "$ID") + if !bsonnav.DSet(control, "DefaultPagePointer", firstID) { + control = append(control, bson.E{Key: "DefaultPagePointer", Value: firstID}) + } + } + if result.parentArr == nil || result.index < 0 || result.index >= len(result.parentArr) { + return fmt.Errorf("cannot add tab pages to %q: its parent slot could not be located", targetRef) + } + result.parentArr[result.index] = control + bsonnav.DSetArray(result.parentDoc, result.parentKey, result.parentArr) + return nil +} + +// serializeTabPages returns the stored form of each tab page, without a list +// marker, by serializing them inside a wrapper tab container. +func (m *Mutator) serializeTabPages(tabPages []*pages.TabPage) ([]any, error) { + if len(tabPages) == 0 { + return nil, fmt.Errorf("no tab pages to insert") + } + wrapper := &pages.TabContainer{TabPages: tabPages} + wrapper.TypeName = "Forms$TabControl" + doc := m.deps.SerializeWidget(wrapper) + var out []any + for _, el := range bsonnav.DGetArrayElements(bsonnav.DGet(doc, "TabPages")) { + if d, ok := el.(bson.D); ok { + out = append(out, d) + } + } + if len(out) != len(tabPages) { + return nil, fmt.Errorf("serialize tab pages: got %d stored tab pages for %d inserted", len(out), len(tabPages)) + } + return out, nil +} diff --git a/mdl/backend/pagemutator/tabpages_test.go b/mdl/backend/pagemutator/tabpages_test.go new file mode 100644 index 0000000000..671759fa7d --- /dev/null +++ b/mdl/backend/pagemutator/tabpages_test.go @@ -0,0 +1,140 @@ +// SPDX-License-Identifier: Apache-2.0 + +package pagemutator + +import ( + "strings" + "testing" + + "go.mongodb.org/mongo-driver/bson" + "go.mongodb.org/mongo-driver/bson/primitive" + + "github.com/mendixlabs/mxcli/mdl/backend" + + "github.com/mendixlabs/mxcli/mdl/backend/bsonnav" + "github.com/mendixlabs/mxcli/mdl/bsonutil" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// tabStubDeps serializes a tab container the way the codec does: a +// Forms$TabControl whose TabPages carry each page's Name. +type tabStubDeps struct{ stubWidgetDeps } + +func (d *tabStubDeps) SerializeWidget(w pages.Widget) bson.D { + tc, ok := w.(*pages.TabContainer) + if !ok { + return d.stubWidgetDeps.SerializeWidget(w) + } + arr := bson.A{int32(3)} + for _, tp := range tc.TabPages { + arr = append(arr, bson.D{ + {Key: "$ID", Value: bsonutil.IDToBsonBinary(string(tp.ID))}, + {Key: "$Type", Value: "Forms$TabPage"}, + {Key: "Name", Value: tp.Name}, + {Key: "Widgets", Value: bson.A{int32(2)}}, + }) + } + return bson.D{{Key: "$Type", Value: "Forms$TabControl"}, {Key: "TabPages", Value: arr}} +} + +func newTabPage(name string) *pages.TabPage { + return &pages.TabPage{BaseElement: model.BaseElement{ID: model.ID(types.GenerateID()), TypeName: "Forms$TabPage"}, Name: name} +} + +func tabPageNames(t *testing.T, m *Mutator) []string { + t.Helper() + var out []string + for _, el := range bsonnav.DGetArrayElements(bsonnav.DGet(findBsonWidget(m.rawData, "tabControl").widget, "TabPages")) { + out = append(out, bsonnav.DGetString(el.(bson.D), "Name")) + } + return out +} + +// mendixlabs/mxcli#1215: a tab page is added to the control's TabPages list — +// appended for INTO the control, next to the sibling for BEFORE/AFTER a tab +// page — and the control keeps the default page it had. +func TestInsertTabPages_Positions(t *testing.T) { + for _, tc := range []struct { + target, position string + want string + }{ + {"tabControl", "into", "tabPage2,tpNew"}, + {"tabPage2", "after", "tabPage2,tpNew"}, + {"tabPage2", "before", "tpNew,tabPage2"}, + } { + t.Run(tc.position, func(t *testing.T) { + m := New(makeTabControlPage(), model.ID("page-1"), &tabStubDeps{}) + before := bsonnav.DGet(findBsonWidget(m.rawData, "tabControl").widget, "DefaultPagePointer") + if err := m.InsertTabPages(tc.target, toPos(tc.position), []*pages.TabPage{newTabPage("tpNew")}); err != nil { + t.Fatalf("insert %s %s: %v", tc.position, tc.target, err) + } + if got := strings.Join(tabPageNames(t, m), ","); got != tc.want { + t.Errorf("TabPages = %s, want %s", got, tc.want) + } + ctl := findBsonWidget(m.rawData, "tabControl").widget + if arr := bsonnav.ToBsonA(bsonnav.DGet(ctl, "TabPages")); !isListMarker(arr[0]) { + t.Error("TabPages lost its list marker") + } + if after := bsonnav.DGet(ctl, "DefaultPagePointer"); !equalIDs(after, before) { + t.Errorf("DefaultPagePointer changed from %v to %v", before, after) + } + if r := findBsonWidget(m.rawData, "tpNew"); r == nil || r.parentKey != "TabPages" { + t.Error("the new tab page does not resolve inside TabPages") + } + }) + } +} + +func toPos(s string) backend.InsertPosition { return backend.InsertPosition(s) } + +// equalIDs compares two stored pointers; nil equals nil. +func equalIDs(a, b any) bool { + ab, aok := a.(primitive.Binary) + bb, bok := b.(primitive.Binary) + if !aok || !bok { + return a == nil && b == nil + } + return bsonutil.BsonBinaryToID(ab) == bsonutil.BsonBinaryToID(bb) +} + +// An empty tab container gains the inserted page as its default. +func TestInsertTabPages_EmptyControlGetsDefault(t *testing.T) { + m := New(makeRawPage(bson.D{ + {Key: "$Type", Value: "Forms$TabControl"}, + {Key: "Name", Value: "tabControl"}, + {Key: "DefaultPagePointer", Value: nil}, + {Key: "TabPages", Value: bson.A{int32(3)}}, + }), model.ID("page-1"), &tabStubDeps{}) + if err := m.InsertTabPages("tabControl", "into", []*pages.TabPage{newTabPage("tpNew")}); err != nil { + t.Fatal(err) + } + ctl := findBsonWidget(m.rawData, "tabControl").widget + newID := bsonnav.DGet(findBsonWidget(m.rawData, "tpNew").widget, "$ID") + if !equalIDs(bsonnav.DGet(ctl, "DefaultPagePointer"), newID) { + t.Errorf("DefaultPagePointer = %v, want the new page %v", bsonnav.DGet(ctl, "DefaultPagePointer"), newID) + } +} + +// A tab page belongs only in a TabPages list: not inside a tab page, and not +// next to an ordinary widget. +func TestInsertTabPages_Refusals(t *testing.T) { + for _, tc := range []struct{ target, position, want string }{ + {"tabPage2", "into", "insert after tabPage2"}, + {"inner", "after", "siblings are tab pages"}, + {"inner", "into", "only be added to a tab container"}, + {"nosuch", "into", "not found"}, + } { + t.Run(tc.target+"/"+tc.position, func(t *testing.T) { + m := New(makeTabControlPage(), model.ID("page-1"), &tabStubDeps{}) + err := m.InsertTabPages(tc.target, toPos(tc.position), []*pages.TabPage{newTabPage("tpNew")}) + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Fatalf("err = %v, want one containing %q", err, tc.want) + } + if got := tabPageNames(t, m); len(got) != 1 { + t.Errorf("TabPages changed to %v by a refused insert", got) + } + }) + } +} diff --git a/mdl/executor/check_declared_entity_types_pedapp_test.go b/mdl/executor/check_declared_entity_types_pedapp_test.go new file mode 100644 index 0000000000..7dbd23fad4 --- /dev/null +++ b/mdl/executor/check_declared_entity_types_pedapp_test.go @@ -0,0 +1,101 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" +) + +// A declared type — a flow's parameter or return type, a page's or snippet's +// parameter — that names an entity the project does not have passed +// `check --references`, for System and user modules alike: +// +// create microflow M.F ($p: System.Nope) begin end; -> Check passed +// create microflow M.F ($p: M.Nope) begin end; -> Check passed +// create page M.P (params: ($x: System.Nope), …) { … }; -> Check passed +// +// Measured on Evora Factory Management (10.24.15). exec refuses the user-module +// shapes and every page parameter ("entity … not found") after the statements +// before it are written; a System one in a flow signature it writes by name, +// leaving a reference nothing resolves. The body of a flow (retrieve, create), +// an association's endpoints and an EXTENDS target were already resolved — the +// signature was the one place a type name went unchecked. +// +// Run on the real path: the parsed script through ValidateProgram and the +// executor's reference check, against the Studio Pro-authored PedApp fixture, +// whose System module is the virtual one the modelsdk backend builds. +func TestCheckDeclaredEntityTypes_UnknownEntityIsRefused(t *testing.T) { + cases := []struct { + name, src, missing string + }{ + {"association FROM System.Nope (the reported shape)", + "create association ModT.Nope_Thing from System.Nope to ModT.Thing;", "System.Nope"}, + {"microflow parameter, System", + "create microflow ModT.F1 ($p: System.Nope) begin end;", "System.Nope"}, + {"microflow parameter, user module", + "create microflow ModT.F1 ($p: ModT.Nope) begin end;", "ModT.Nope"}, + {"microflow list parameter", + "create microflow ModT.F1 ($p: list of System.Nope) begin end;", "System.Nope"}, + {"microflow return type", + "create microflow ModT.F1 () returns System.Nope begin return empty; end;", "System.Nope"}, + {"nanoflow parameter", + "create nanoflow ModT.N1 ($p: System.Nope) begin end;", "System.Nope"}, + {"page parameter", + "create page ModT.P1 (params: ($x: System.Nope), title: 'x', layout: Atlas_Core.Atlas_Default) { };", + "System.Nope"}, + {"snippet parameter", + "create snippet ModT.S1 (params: ($x: ModT.Nope)) { };", "ModT.Nope"}, + } + + exec, _, dir := openPedAppCopy(t) + if err := agreeExec(t, exec, "mdl 1;\ncreate module ModT;\ncreate persistent entity ModT.Thing (Name: String(100));"); err != nil { + t.Fatalf("setup: %v", err) + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := strings.Join(agreeCheck(t, exec, dir, "mdl 1;\n"+c.src), "\n") + if !strings.Contains(got, c.missing) { + t.Fatalf("check --references must name the unknown entity %s, reported:\n%s", c.missing, got) + } + }) + } +} + +// The controls: real System entities and enumerations, and an entity the +// script itself creates, must keep passing. A reference check louder than the +// model is a blocker, not a safety net. +func TestCheckDeclaredEntityTypes_ResolvableTypesPass(t *testing.T) { + cases := []struct{ name, src string }{ + {"association to System.User", + "create association ModT.Thing_User from ModT.Thing to System.User;"}, + {"microflow parameter System.User", + "create microflow ModT.F1 ($u: System.User) begin end;"}, + {"microflow list parameter of System.FileDocument", + "create microflow ModT.F1 ($l: list of System.FileDocument) begin end;"}, + {"microflow returning System.Image", + "create microflow ModT.F1 () returns System.Image begin return empty; end;"}, + {"microflow parameter of a System enumeration", + "create microflow ModT.F1 ($d: System.DeviceType) begin end;"}, + {"microflow parameter of a stored user entity", + "create microflow ModT.F1 ($t: ModT.Thing) begin end;"}, + {"nanoflow parameter System.Session", + "create nanoflow ModT.N1 ($s: System.Session) begin end;"}, + {"page parameter System.User", + "create page ModT.P1 (params: ($x: System.User), title: 'x', layout: Atlas_Core.Atlas_Default) { };"}, + {"parameter of an entity the script creates", + "create persistent entity ModT.Later (Name: String(10));\ncreate microflow ModT.F1 ($l: ModT.Later) begin end;"}, + } + + exec, _, dir := openPedAppCopy(t) + if err := agreeExec(t, exec, "mdl 1;\ncreate module ModT;\ncreate persistent entity ModT.Thing (Name: String(100));"); err != nil { + t.Fatalf("setup: %v", err) + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := agreeCheck(t, exec, dir, "mdl 1;\n"+c.src); len(got) != 0 { + t.Fatalf("a resolvable type was refused:\n%s", strings.Join(got, "\n")) + } + }) + } +} diff --git a/mdl/executor/cmd_alter_page.go b/mdl/executor/cmd_alter_page.go index 4498bacc79..4dbdd3b1d3 100644 --- a/mdl/executor/cmd_alter_page.go +++ b/mdl/executor/cmd_alter_page.go @@ -456,6 +456,25 @@ func applyInsertWidgetMutator(ctx *ExecContext, mutator backend.PageMutator, op into := strings.EqualFold(op.Position, "INTO") entityCtx, _ := alterEntityContext(ctx, mutator, op.Target.Widget, into, moduleName, moduleID) + // Special path: adding tab pages to a tab container (#1215). A tab page lives + // in the container's TabPages list and is not a widget, so the builder + // refuses one on its own ("tabpage must be a direct child of tabcontainer") + // and InsertWidget would put it in a widget list. INTO targets the container, + // BEFORE/AFTER a sibling tab page; the mutator checks which the target is. + if allTabPages(op.Widgets) { + tabPages, err := buildTabPagesFromAST(ctx, op.Widgets, moduleName, moduleID, entityCtx, mutator) + if err != nil { + return mdlerrors.NewBackend("build tab pages", err) + } + return mutator.InsertTabPages(op.Target.Widget, backend.InsertPosition(op.Position), tabPages) + } + if hasTabPage(op.Widgets) { + return mdlerrors.NewValidation( + "mixing `tabpage` blocks with ordinary widgets in one INSERT is not supported: " + + "tab pages go in a tab container's list of pages, widgets inside a tab page. " + + "Use one INSERT for the tab pages and another for the widgets") + } + // Build new widgets from AST widgets, err := buildWidgetsFromAST(ctx, op.Widgets, moduleName, moduleID, entityCtx, mutator) if err != nil { @@ -873,13 +892,71 @@ func resolveDataSourceFlowEntity(ctx *ExecContext, moduleName string, moduleID m // excludeFromScope removes named widgets from the duplicate-detection scope, // used when replacing a widget so the new one may reuse the target's name. func buildWidgetsFromAST(ctx *ExecContext, widgets []*ast.WidgetV3, moduleName string, moduleID model.ID, entityContext string, mutator backend.PageMutator, excludeFromScope ...string) ([]pages.Widget, error) { + pb := newAlterPageBuilder(ctx, moduleName, moduleID, entityContext, mutator, excludeFromScope...) + + var result []pages.Widget + for _, w := range widgets { + widget, err := pb.buildWidgetV3(w) + if err != nil { + return nil, mdlerrors.NewBackend("build widget "+w.Name, err) + } + if widget == nil { + continue + } + result = append(result, widget) + } + return result, nil +} + +// buildTabPagesFromAST builds the tab pages an INSERT adds to a tab container, +// with the same builder CREATE PAGE uses for a tab container's children. +func buildTabPagesFromAST(ctx *ExecContext, nodes []*ast.WidgetV3, moduleName string, moduleID model.ID, entityContext string, mutator backend.PageMutator) ([]*pages.TabPage, error) { + pb := newAlterPageBuilder(ctx, moduleName, moduleID, entityContext, mutator) + out := make([]*pages.TabPage, 0, len(nodes)) + for _, node := range nodes { + tp, err := pb.buildTabPageV3(node) + if err != nil { + return nil, mdlerrors.NewBackend("build tab page "+node.Name, err) + } + out = append(out, tp) + } + return out, nil +} + +// allTabPages reports whether every inserted node is a tab page. +func allTabPages(widgets []*ast.WidgetV3) bool { + if len(widgets) == 0 { + return false + } + for _, w := range widgets { + if !strings.EqualFold(w.Type, "tabpage") { + return false + } + } + return true +} + +// hasTabPage reports whether ANY inserted node is a tab page, so a mixed insert +// is refused rather than sending the tab pages down the widget path. +func hasTabPage(widgets []*ast.WidgetV3) bool { + for _, w := range widgets { + if strings.EqualFold(w.Type, "tabpage") { + return true + } + } + return false +} + +// newAlterPageBuilder returns a page builder scoped to the stored page an ALTER +// edits: its parameters, its widget names and its local variables. +func newAlterPageBuilder(ctx *ExecContext, moduleName string, moduleID model.ID, entityContext string, mutator backend.PageMutator, excludeFromScope ...string) *pageBuilder { paramScope, paramEntityNames := mutator.ParamScope() widgetScope := mutator.WidgetScope() for _, name := range excludeFromScope { delete(widgetScope, name) } - pb := &pageBuilder{ + return &pageBuilder{ ctx: ctx, backend: ctx.Backend, moduleID: moduleID, @@ -895,17 +972,4 @@ func buildWidgetsFromAST(ctx *ExecContext, widgets []*ast.WidgetV3, moduleName s localVariables: storedPageVariables(mutator), isSnippet: mutator.ContainerType() == backend.ContainerSnippet, } - - var result []pages.Widget - for _, w := range widgets { - widget, err := pb.buildWidgetV3(w) - if err != nil { - return nil, mdlerrors.NewBackend("build widget "+w.Name, err) - } - if widget == nil { - continue - } - result = append(result, widget) - } - return result, nil } diff --git a/mdl/executor/cmd_alter_page_tabpage_test.go b/mdl/executor/cmd_alter_page_tabpage_test.go new file mode 100644 index 0000000000..a6a01f6b09 --- /dev/null +++ b/mdl/executor/cmd_alter_page_tabpage_test.go @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +func tabPageNode(name, caption string) *ast.WidgetV3 { + return &ast.WidgetV3{ + Type: "tabpage", + Name: name, + Properties: map[string]any{"Caption": caption}, + Children: []*ast.WidgetV3{{ + Type: "dynamictext", Name: "txt" + name, Properties: map[string]any{"Content": "two"}, + }}, + } +} + +// mendixlabs/mxcli#1215: `insert after tpOne { tabpage tpTwo … }` and +// `insert into tabsMain { tabpage tpTwo … }` both failed with "failed to build +// widget tpTwo: tabpage must be a direct child of tabcontainer" — the widgets of +// an INSERT were built one by one, and a lone tab page is refused by the +// builder. Both forms must build the tab page and hand it to InsertTabPages +// with the position and target the statement named. +func TestAlterPageInsertTabPage_RoutesToTabPages(t *testing.T) { + for _, tc := range []struct{ position, target string }{ + {"AFTER", "tpOne"}, + {"BEFORE", "tpOne"}, + {"INTO", "tabsMain"}, + } { + t.Run(tc.position, func(t *testing.T) { + var gotTarget string + var gotPos backend.InsertPosition + var gotPages []*pages.TabPage + mutator := &mock.MockPageMutator{ + InsertTabPagesFunc: func(target string, pos backend.InsertPosition, tps []*pages.TabPage) error { + gotTarget, gotPos, gotPages = target, pos, tps + return nil + }, + InsertWidgetFunc: func(string, string, backend.InsertPosition, []pages.Widget) error { + t.Error("a tab page went through InsertWidget — it would land in a widget list") + return nil + }, + } + err := alterPageWith(t, mutator, &ast.InsertWidgetOp{ + Position: tc.position, + Target: ast.WidgetRef{Widget: tc.target}, + Widgets: []*ast.WidgetV3{tabPageNode("tpTwo", "Two")}, + }) + if err != nil { + t.Fatalf("insert %s %s { tabpage tpTwo }: %v", tc.position, tc.target, err) + } + if gotTarget != tc.target || !strings.EqualFold(string(gotPos), tc.position) { + t.Errorf("InsertTabPages(%q, %q), want (%q, %q)", gotTarget, gotPos, tc.target, tc.position) + } + if len(gotPages) != 1 { + t.Fatalf("got %d tab pages, want 1", len(gotPages)) + } + tp := gotPages[0] + if tp.Name != "tpTwo" || tp.TypeName != "Forms$TabPage" { + t.Errorf("tab page = %s %q, want Forms$TabPage tpTwo", tp.TypeName, tp.Name) + } + if tp.Caption == nil || !captionHas(tp.Caption.Translations, "Two") { + t.Errorf("caption = %+v, want Two", tp.Caption) + } + if len(tp.Widgets) != 1 || tp.Widgets[0].GetName() != "txttpTwo" { + t.Errorf("tab page widgets = %d, want the dynamic text", len(tp.Widgets)) + } + }) + } +} + +func captionHas(tr map[string]string, want string) bool { + for _, v := range tr { + if v == want { + return true + } + } + return false +} + +// A tab page and an ordinary widget go to different lists; one INSERT cannot +// mean both, so it is refused rather than half-applied. +func TestAlterPageInsertTabPage_MixedRefused(t *testing.T) { + mutator := &mock.MockPageMutator{ + InsertTabPagesFunc: func(string, backend.InsertPosition, []*pages.TabPage) error { + t.Error("InsertTabPages called for a mixed insert") + return nil + }, + } + err := alterPageWith(t, mutator, &ast.InsertWidgetOp{ + Position: "AFTER", + Target: ast.WidgetRef{Widget: "tpOne"}, + Widgets: []*ast.WidgetV3{ + tabPageNode("tpTwo", "Two"), + {Type: "dynamictext", Name: "plain", Properties: map[string]any{"Content": "x"}}, + }, + }) + assertError(t, err) + assertContainsStr(t, err.Error(), "mixing `tabpage`") +} diff --git a/mdl/executor/cmd_microflows_builder_annotations.go b/mdl/executor/cmd_microflows_builder_annotations.go index e222f6100b..8cd4c2ceed 100644 --- a/mdl/executor/cmd_microflows_builder_annotations.go +++ b/mdl/executor/cmd_microflows_builder_annotations.go @@ -57,6 +57,13 @@ func (fb *flowBuilder) mergeStatementAnnotations(stmt ast.MicroflowStatement) { if len(ann.FreeNotes) > 0 { fb.pendingAnnotations.FreeNotes = append(fb.pendingAnnotations.FreeNotes, ann.FreeNotes...) } + // Missing until #1328: the parser read @excluded and applyAnnotations wrote + // it, but this hand-written copy skipped it, so every excluded activity was + // written enabled. TestMergeStatementAnnotationsCopiesEveryField guards the + // next field added to ActivityAnnotations. + if ann.Excluded { + fb.pendingAnnotations.Excluded = true + } if ann.Anchor != nil { fb.pendingAnnotations.Anchor = ann.Anchor } diff --git a/mdl/executor/cmd_published_rest.go b/mdl/executor/cmd_published_rest.go index 6533f0288b..6d2f634bb5 100644 --- a/mdl/executor/cmd_published_rest.go +++ b/mdl/executor/cmd_published_rest.go @@ -113,7 +113,14 @@ func describePublishedRestService(ctx *ExecContext, name ast.QualifiedName) erro if svc.ServiceName != "" { fmt.Fprintf(ctx.Output, ",\n ServiceName: %s", mdlQuoted(svc.ServiceName)) } + authProp, authNote, hasAuth := publishedRestAuthenticationProperty(svc) + if hasAuth { + fmt.Fprintf(ctx.Output, ",\n %s", authProp) + } fmt.Fprintln(ctx.Output, "\n)") + if authNote != "" { + fmt.Fprintf(ctx.Output, "-- Authentication is stored as %s, which MDL cannot state; executing this keeps it.\n", authNote) + } if len(svc.Resources) > 0 { fmt.Fprintln(ctx.Output, "{") @@ -163,6 +170,88 @@ func describePublishedRestService(ctx *ExecContext, name ast.QualifiedName) erro return mdlerrors.NewNotFound("published rest service", name.String()) } +// applyPublishedRestAuthentication sets a service's authentication from the +// statement; nil (not stated) leaves it as it is. Methods and microflow are +// replaced together, so a list without `microflow` clears the microflow, as +// Studio Pro does when Custom is unticked. +// +// The microflow is checked against what Mendix accepts before anything is +// written (MDL-REST04). +func applyPublishedRestAuthentication(ctx *ExecContext, svc *model.PublishedRestService, auth *ast.PublishedRestAuthentication) error { + if auth == nil { + return nil + } + if auth.Microflow != "" { + mf, ok := liveMicroflowsByQualifiedName(ctx)[auth.Microflow] + if !ok { + return mdlerrors.NewValidationf("MDL-REST04: authentication microflow not found: %s", auth.Microflow) + } + if problem := publishedRestAuthMicroflowProblem(mf); problem != "" { + return mdlerrors.NewValidationf("MDL-REST04: authentication microflow %s %s", auth.Microflow, problem) + } + } + svc.AuthenticationTypes = append([]string(nil), auth.Methods...) + svc.AuthenticationMicroflow = auth.Microflow + return nil +} + +// publishedRestAuthMicroflowProblem says why Mendix would reject mf as a +// published REST service's custom-authentication microflow, or "" when it +// would not. Measured with mx check on 11.14.0 (mendixlabs/mxcli#1331): the +// microflow returns System.User (CE0334), and each parameter is one Mendix +// supplies, matched by type and not by name (CE0336). No parameters is fine. +func publishedRestAuthMicroflowProblem(mf *microflows.Microflow) string { + if ot, ok := mf.ReturnType.(*microflows.ObjectType); !ok || ot.EntityQualifiedName != "System.User" { + return "must return System.User (CE0334)" + } + for _, p := range mf.Parameters { + ot, ok := p.Type.(*microflows.ObjectType) + if !ok || (ot.EntityQualifiedName != "System.HttpRequest" && ot.EntityQualifiedName != "System.HttpResponse") { + return fmt.Sprintf("has parameter $%s, which Mendix does not supply to an authentication microflow: "+ + "its parameters can only be a System.HttpRequest and a System.HttpResponse (CE0336)", p.Name) + } + } + return "" +} + +// publishedRestAuthenticationProperty renders a service's stored +// authentication as the `Authentication:` property, in the stored order. ok is +// false when there is nothing to print (no methods: the creation default) or +// when the stored value has no MDL spelling — a method other than basic, +// session and microflow, or a microflow without the Microflow method or the +// reverse. note then says what is stored, for a comment: leaving the property +// out keeps the stored value when the output is executed. +func publishedRestAuthenticationProperty(svc *model.PublishedRestService) (prop, note string, ok bool) { + if len(svc.AuthenticationTypes) == 0 && svc.AuthenticationMicroflow == "" { + return "", "", false + } + parts := make([]string, 0, len(svc.AuthenticationTypes)) + spellable, hasMicroflow := true, false + for _, t := range svc.AuthenticationTypes { + switch t { + case "Basic", "Session": + parts = append(parts, strings.ToLower(t)) + case "Microflow": + hasMicroflow = true + if svc.AuthenticationMicroflow == "" { + spellable = false + continue + } + parts = append(parts, "microflow "+svc.AuthenticationMicroflow) + default: + spellable = false + } + } + if spellable && hasMicroflow == (svc.AuthenticationMicroflow != "") && len(parts) > 0 { + return "Authentication: (" + strings.Join(parts, ", ") + ")", "", true + } + note = "methods [" + strings.Join(svc.AuthenticationTypes, ", ") + "]" + if svc.AuthenticationMicroflow != "" { + note += ", microflow " + svc.AuthenticationMicroflow + } + return "", note, false +} + // publishedRestBindingClauses prints an operation's mapping bindings and its // commit option, in the grammar's order. Commit is printed when it is not // "Yes", the value exec writes when the statement has no commit clause. @@ -284,6 +373,13 @@ func execCreatePublishedRestService(ctx *ExecContext, s *ast.CreatePublishedRest svc.AllowedRoles = existing.AllowedRoles // Excluded is model state, not script state (#914). svc.Excluded = existing.Excluded + // An unstated authentication keeps the stored one; stating it below + // replaces it (mendixlabs/mxcli#1331, ako/mxcli#571). + svc.AuthenticationTypes = existing.AuthenticationTypes + svc.AuthenticationMicroflow = existing.AuthenticationMicroflow + } + if err := applyPublishedRestAuthentication(ctx, svc, s.Authentication); err != nil { + return err } for _, resDef := range s.Resources { @@ -595,9 +691,12 @@ func execAlterPublishedRestService(ctx *ExecContext, s *ast.AlterPublishedRestSe case "servicename": svc.ServiceName = val default: - return mdlerrors.NewUnsupported(fmt.Sprintf("unknown published rest service property: %s (allowed: Path, Version, ServiceName)", key)) + return mdlerrors.NewUnsupported(fmt.Sprintf("unknown published rest service property: %s (allowed: Path, Version, ServiceName, Authentication)", key)) } } + if err := applyPublishedRestAuthentication(ctx, svc, a.Authentication); err != nil { + return err + } case *ast.PublishedRestAddResourceAction: // Reject duplicate resource names diff --git a/mdl/executor/cmd_published_rest_auth_test.go b/mdl/executor/cmd_published_rest_auth_test.go new file mode 100644 index 0000000000..c9a016b9c6 --- /dev/null +++ b/mdl/executor/cmd_published_rest_auth_test.go @@ -0,0 +1,242 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "reflect" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/model" +) + +// storedAuthService is a service as Studio Pro stores it, with the given +// authentication. The methods are ako/TestApp's (mendixlabs/mxcli#1331). +func storedAuthService(types []string, mf string) *model.PublishedRestService { + s := &model.PublishedRestService{ + Name: "Orders", Path: "rest/orders/v1", Version: "1.0.0", + AuthenticationTypes: types, AuthenticationMicroflow: mf, + Resources: []*model.PublishedRestResource{{ + Name: "orders", + Operations: []*model.PublishedRestOperation{{HTTPMethod: "GET", Path: "status", Microflow: "RestQ.GetStatus"}}, + }}, + } + s.ID = nextID("prs") + return s +} + +func authOf(s *model.PublishedRestService) string { + if s == nil { + return "" + } + return strings.Join(s.AuthenticationTypes, ",") + " / " + s.AuthenticationMicroflow +} + +func TestCreatePublishedRestService_WritesAuthentication(t *testing.T) { + f, ctx, _ := newPublishedRestFixture(t) + assertNoError(t, execPublishedRest(t, ctx, `create published rest service RestQ.Orders ( + Path: 'rest/orders/v1', + Authentication: (microflow RestQ.Authenticate, session) +) { + resource 'orders' { get 'status' microflow RestQ.GetStatus; } +};`)) + if got, want := authOf(f.created), "Microflow,Session / RestQ.Authenticate"; got != want { + t.Errorf("authentication = %q, want %q", got, want) + } +} + +// A statement that does not state authentication keeps the stored setting. +// Before #1331 the writer carried it; now the executor must, or every +// `create or modify` and `alter` of a Studio Pro service would switch its +// authentication off (the ako/mxcli#571 regression). +func TestPublishedRestService_UnstatedAuthenticationIsKept(t *testing.T) { + for name, src := range map[string]string{ + "create or modify": `create or modify published rest service RestQ.Orders (Path: 'rest/orders/v2') { + resource 'orders' { get 'status' microflow RestQ.GetStatus; } +};`, + "alter set list": `alter published rest service RestQ.Orders set (Version: '2.0.0');`, + "alter set assign": `alter published rest service RestQ.Orders set Version = '2.0.0';`, + } { + t.Run(name, func(t *testing.T) { + stored := storedAuthService([]string{"Basic", "Session", "Microflow"}, "RestQ.Authenticate") + f, ctx, _ := newPublishedRestFixture(t, stored) + assertNoError(t, execPublishedRest(t, ctx, src)) + if got, want := authOf(f.updated), "Basic,Session,Microflow / RestQ.Authenticate"; got != want { + t.Errorf("authentication = %q, want %q (the stored setting)", got, want) + } + }) + } +} + +// Stating it replaces methods and microflow together: a method list without +// `microflow` clears the microflow, as Studio Pro does when Custom is unticked. +func TestPublishedRestService_StatedAuthenticationReplaces(t *testing.T) { + cases := []struct{ name, src, want string }{ + {"create or modify none", `create or modify published rest service RestQ.Orders (Path: 'rest/orders/v1', Authentication: none) { + resource 'orders' { get 'status' microflow RestQ.GetStatus; } +};`, " / "}, + {"alter custom off", `alter published rest service RestQ.Orders set (Authentication: (basic, session));`, "Basic,Session / "}, + {"alter none", `alter published rest service RestQ.Orders set (Authentication: none);`, " / "}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + stored := storedAuthService([]string{"Basic", "Session", "Microflow"}, "RestQ.Authenticate") + f, ctx, _ := newPublishedRestFixture(t, stored) + assertNoError(t, execPublishedRest(t, ctx, c.src)) + if got := authOf(f.updated); got != c.want { + t.Errorf("authentication = %q, want %q", got, c.want) + } + }) + } +} + +// describe prints the stored methods in the stored order, and executing its +// output writes the same setting back — for every state measured on TestApp. +// The control is the order: TestApp's MicroflowSession service stores +// Microflow before Session, so a describe that sorted would fail here. +func TestDescribePublishedRestService_AuthenticationRoundTrips(t *testing.T) { + cases := []struct { + name string + types []string + mf string + line string + }{ + {"all three", []string{"Basic", "Session", "Microflow"}, "RestQ.Authenticate", "Authentication: (basic, session, microflow RestQ.Authenticate)"}, + {"microflow first", []string{"Microflow", "Session"}, "RestQ.Authenticate", "Authentication: (microflow RestQ.Authenticate, session)"}, + {"custom off", []string{"Basic", "Session"}, "", "Authentication: (basic, session)"}, + {"none", nil, "", ""}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + stored := storedAuthService(c.types, c.mf) + f, ctx, out := newPublishedRestFixture(t, stored) + assertNoError(t, describePublishedRestService(ctx, ast.QualifiedName{Module: "RestQ", Name: "Orders"})) + text := out.String() + if c.line != "" && !strings.Contains(text, c.line) { + t.Fatalf("describe lacks %q:\n%s", c.line, text) + } + if c.line == "" && strings.Contains(text, "Authentication") { + t.Fatalf("describe prints the default (none):\n%s", text) + } + // Replay onto a service whose stored setting differs, so a dropped + // or reordered property cannot pass by carrying the old value. + other := storedAuthService([]string{"Session"}, "") + other.ContainerID = f.mod.ID + f.stored = []*model.PublishedRestService{other} + stmt := text[:strings.Index(text, "};")+2] + assertNoError(t, execPublishedRest(t, ctx, stmt)) + if c.line == "" { + // none is the creation default and is not printed, so replaying + // it keeps whatever is stored — the documented meaning of an + // unstated property. + return + } + if !reflect.DeepEqual(f.updated.AuthenticationTypes, c.types) || f.updated.AuthenticationMicroflow != c.mf { + t.Errorf("replayed as %q, want %v / %q", authOf(f.updated), c.types, c.mf) + } + }) + } +} + +// A stored value MDL cannot spell is not printed as the property, so replaying +// the output keeps it rather than writing something else. +func TestDescribePublishedRestService_UnspellableAuthenticationIsAComment(t *testing.T) { + for name, st := range map[string]*model.PublishedRestService{ + "guest": storedAuthService([]string{"Guest"}, ""), + "microflow, no method": storedAuthService([]string{"Basic"}, "RestQ.Authenticate"), + "method, no microflow": storedAuthService([]string{"Microflow"}, ""), + } { + t.Run(name, func(t *testing.T) { + _, ctx, out := newPublishedRestFixture(t, st) + assertNoError(t, describePublishedRestService(ctx, ast.QualifiedName{Module: "RestQ", Name: "Orders"})) + text := out.String() + if strings.Contains(text, " Authentication:") { + t.Errorf("printed an unspellable setting as the property:\n%s", text) + } + if !strings.Contains(text, "-- Authentication") { + t.Errorf("no comment for the stored setting:\n%s", text) + } + }) + } +} + +func TestAlterPublishedRestService_SetListRefusesFolder(t *testing.T) { + _, ctx, _ := newPublishedRestFixture(t, storedAuthService(nil, "")) + err := execPublishedRest(t, ctx, `alter published rest service RestQ.Orders set (Folder: 'Apis');`) + if err == nil || !strings.Contains(err.Error(), "Folder") { + t.Fatalf("err = %v, want a refusal naming Folder", err) + } +} + +// The custom-authentication microflow must be one Mendix accepts, measured with +// mx check on 11.14.0: it returns System.User (CE0334), and every parameter is +// one Mendix supplies, System.HttpRequest or System.HttpResponse, matched by +// type and not by name (CE0336). No parameters at all is accepted. +func TestPublishedRestService_AuthenticationMicroflowSignature(t *testing.T) { + cases := []struct{ mf, want string }{ + {"RestQ.Authenticate", ""}, + {"RestQ.AuthenticateBoth", ""}, + {"RestQ.AuthenticateNoParams", ""}, + {"RestQ.AuthenticateBool", "CE0334"}, + {"RestQ.AuthenticateToken", "CE0336"}, + {"RestQ.NoSuchMicroflow", "not found"}, + } + for _, c := range cases { + for name, src := range map[string]string{ + "create": `create published rest service RestQ.Orders (Path: 'rest/orders/v1', Authentication: (microflow ` + c.mf + `)) { + resource 'orders' { get 'status' microflow RestQ.GetStatus; } +};`, + "alter": `alter published rest service RestQ.Orders set (Authentication: (session, microflow ` + c.mf + `));`, + } { + t.Run(c.mf+"/"+name, func(t *testing.T) { + var stored []*model.PublishedRestService + if name == "alter" { + stored = append(stored, storedAuthService(nil, "")) + } + f, ctx, _ := newPublishedRestFixture(t, stored...) + err := execPublishedRest(t, ctx, src) + if c.want == "" { + assertNoError(t, err) + return + } + if err == nil || !strings.Contains(err.Error(), c.want) || !strings.Contains(err.Error(), c.mf) { + t.Fatalf("err = %v, want a refusal naming %s and %s", err, c.mf, c.want) + } + if f.created != nil || f.updated != nil { + t.Errorf("wrote the service despite the refusal") + } + }) + } + } +} + +// check --references predicts exec's MDL-REST04: a missing microflow and a +// project microflow Mendix would reject are reported before anything runs. A +// microflow the script itself creates is known by name only, so its signature +// is left to exec. +func TestReferences_PublishedRestAuthenticationMicroflow(t *testing.T) { + cases := []struct{ src, want string }{ + {`create published rest service RestQ.Orders (Path: 'p', Authentication: (microflow RestQ.Authenticate)) { resource 'r' { get '' microflow RestQ.GetStatus; } };`, ""}, + {`create published rest service RestQ.Orders (Path: 'p', Authentication: (microflow RestQ.Nope)) { resource 'r' { get '' microflow RestQ.GetStatus; } };`, "not found"}, + {`create published rest service RestQ.Orders (Path: 'p', Authentication: (microflow RestQ.AuthenticateBool)) { resource 'r' { get '' microflow RestQ.GetStatus; } };`, "CE0334"}, + {`alter published rest service RestQ.Orders set (Authentication: (microflow RestQ.AuthenticateToken));`, "CE0336"}, + {`create microflow RestQ.Later () returns System.User begin return empty; end; +create published rest service RestQ.Orders (Path: 'p', Authentication: (microflow RestQ.Later)) { resource 'r' { get '' microflow RestQ.GetStatus; } };`, ""}, + } + for _, c := range cases { + t.Run(c.want+"/"+c.src[:30], func(t *testing.T) { + _, ctx, _ := newPublishedRestFixture(t, storedAuthService(nil, "")) + errs := enumErrors(validateProgram(ctx, parseMDL(t, c.src))) + if c.want == "" { + if strings.Contains(errs, "MDL-REST04") { + t.Fatalf("unexpected: %s", errs) + } + return + } + if !strings.Contains(errs, "MDL-REST04") || !strings.Contains(errs, c.want) { + t.Fatalf("errors %q, want MDL-REST04 with %q", errs, c.want) + } + }) + } +} diff --git a/mdl/executor/cmd_published_rest_params_test.go b/mdl/executor/cmd_published_rest_params_test.go index 011a5c0fdb..6f70fe77c6 100644 --- a/mdl/executor/cmd_published_rest_params_test.go +++ b/mdl/executor/cmd_published_rest_params_test.go @@ -47,6 +47,23 @@ func newPublishedRestFixture(t *testing.T, stored ...*model.PublishedRestService mf("PutFile", param("file", µflows.ObjectType{EntityQualifiedName: "RestQ.Upload"})), mf("PutMany", param("items", µflows.ListType{EntityQualifiedName: "RestQ.Item"})), } + // Custom-authentication microflows, the shapes measured with mx check on + // 11.14.0 (mendixlabs/mxcli#1331). + user := µflows.ObjectType{EntityQualifiedName: "System.User"} + authMf := func(name string, ret microflows.DataType, params ...*microflows.MicroflowParameter) *microflows.Microflow { + m := mf(name, params...) + m.ReturnType = ret + return m + } + mfs = append(mfs, + authMf("Authenticate", user, param("HttpRequest", µflows.ObjectType{EntityQualifiedName: "System.HttpRequest"})), + authMf("AuthenticateBoth", user, + param("Req", µflows.ObjectType{EntityQualifiedName: "System.HttpRequest"}), + param("Resp", µflows.ObjectType{EntityQualifiedName: "System.HttpResponse"})), + authMf("AuthenticateNoParams", user), + authMf("AuthenticateBool", µflows.BooleanType{}, param("HttpRequest", µflows.ObjectType{EntityQualifiedName: "System.HttpRequest"})), + authMf("AuthenticateToken", user, param("Token", µflows.StringType{})), + ) mb := &mock.MockBackend{ IsConnectedFunc: func() bool { return true }, ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{f.mod}, nil }, diff --git a/mdl/executor/cmd_structure.go b/mdl/executor/cmd_structure.go index dd22d89132..95f17dab13 100644 --- a/mdl/executor/cmd_structure.go +++ b/mdl/executor/cmd_structure.go @@ -67,17 +67,21 @@ func execShowStructure(ctx *ExecContext, s *ast.ShowStmt) error { // structureDepth1JSON emits structure as a JSON table with one row per module // and columns for each element type count. func structureDepth1JSON(ctx *ExecContext, modules []structureModule) error { - entityCounts := queryCountByModule(ctx, "entities") - mfCounts := queryCountByModule(ctx, flowTypeFilter(catalog.MicroflowTypeMicroflow)) - nfCounts := queryCountByModule(ctx, flowTypeFilter(catalog.MicroflowTypeNanoflow)) - pageCounts := queryCountByModule(ctx, "pages") - enumCounts := queryCountByModule(ctx, "enumerations") - snippetCounts := queryCountByModule(ctx, "snippets") - jaCounts := queryCountByModule(ctx, "java_actions") - wfCounts := queryCountByModule(ctx, "workflows") - odataClientCounts := queryCountByModule(ctx, "odata_clients") - odataServiceCounts := queryCountByModule(ctx, "odata_services") - beServiceCounts := queryCountByModule(ctx, "business_event_services") + var qerr error + entityCounts := queryCountByModule(ctx, "entities", &qerr) + mfCounts := queryCountByModule(ctx, flowTypeFilter(catalog.MicroflowTypeMicroflow), &qerr) + nfCounts := queryCountByModule(ctx, flowTypeFilter(catalog.MicroflowTypeNanoflow), &qerr) + pageCounts := queryCountByModule(ctx, "pages", &qerr) + enumCounts := queryCountByModule(ctx, "enumerations", &qerr) + snippetCounts := queryCountByModule(ctx, "snippets", &qerr) + jaCounts := queryCountByModule(ctx, "java_actions", &qerr) + wfCounts := queryCountByModule(ctx, "workflows", &qerr) + odataClientCounts := queryCountByModule(ctx, "odata_clients", &qerr) + odataServiceCounts := queryCountByModule(ctx, "odata_services", &qerr) + beServiceCounts := queryCountByModule(ctx, "business_event_services", &qerr) + if qerr != nil { + return qerr + } constantCounts := countByModuleFromBackend(ctx, "constants") scheduledEventCounts := countByModuleFromBackend(ctx, "scheduled_events") queueCounts := countByModuleFromBackend(ctx, "queues") @@ -189,18 +193,22 @@ func asString(v any) string { // ============================================================================ func structureDepth1(ctx *ExecContext, modules []structureModule) error { + var qerr error // Query counts per module from catalog - entityCounts := queryCountByModule(ctx, "entities") - mfCounts := queryCountByModule(ctx, flowTypeFilter(catalog.MicroflowTypeMicroflow)) - nfCounts := queryCountByModule(ctx, flowTypeFilter(catalog.MicroflowTypeNanoflow)) - pageCounts := queryCountByModule(ctx, "pages") - enumCounts := queryCountByModule(ctx, "enumerations") - snippetCounts := queryCountByModule(ctx, "snippets") - jaCounts := queryCountByModule(ctx, "java_actions") - wfCounts := queryCountByModule(ctx, "workflows") - odataClientCounts := queryCountByModule(ctx, "odata_clients") - odataServiceCounts := queryCountByModule(ctx, "odata_services") - beServiceCounts := queryCountByModule(ctx, "business_event_services") + entityCounts := queryCountByModule(ctx, "entities", &qerr) + mfCounts := queryCountByModule(ctx, flowTypeFilter(catalog.MicroflowTypeMicroflow), &qerr) + nfCounts := queryCountByModule(ctx, flowTypeFilter(catalog.MicroflowTypeNanoflow), &qerr) + pageCounts := queryCountByModule(ctx, "pages", &qerr) + enumCounts := queryCountByModule(ctx, "enumerations", &qerr) + snippetCounts := queryCountByModule(ctx, "snippets", &qerr) + jaCounts := queryCountByModule(ctx, "java_actions", &qerr) + wfCounts := queryCountByModule(ctx, "workflows", &qerr) + odataClientCounts := queryCountByModule(ctx, "odata_clients", &qerr) + odataServiceCounts := queryCountByModule(ctx, "odata_services", &qerr) + beServiceCounts := queryCountByModule(ctx, "business_event_services", &qerr) + if qerr != nil { + return qerr + } // Get constants and scheduled events from backend (no catalog tables) constantCounts := countByModuleFromBackend(ctx, "constants") @@ -272,12 +280,31 @@ func structureDepth1(ctx *ExecContext, modules []structureModule) error { return nil } -// queryCountByModule queries a catalog table and returns a map of module name → count. -func queryCountByModule(ctx *ExecContext, tableAndWhere string) map[string]int { +// structureQuery runs one of the structure overview's catalog queries. They are +// fixed SQL against tables the catalog builder owns, so a failure is a column +// or table drift inside mxcli, never a property of the user's project. They +// used to discard the error, which is how depth 2 and 3 queried a ParentWidget +// column widgets_data has never had and printed every page without its data +// widgets, from the initial commit on. +func structureQuery(ctx *ExecContext, sql string) (*catalog.QueryResult, error) { + result, err := ctx.Catalog.Query(sql) + if err != nil { + return nil, mdlerrors.NewBackend("run structure query "+sql, err) + } + return result, nil +} + +// queryCountByModule queries a catalog table and returns a map of module name → +// count. A failing query sets *errp (if not already set) and yields no counts, +// so a caller can issue its whole batch and check once. +func queryCountByModule(ctx *ExecContext, tableAndWhere string, errp *error) map[string]int { counts := make(map[string]int) sql := fmt.Sprintf("select ModuleName, count(*) from %s GROUP by ModuleName", tableAndWhere) - result, err := ctx.Catalog.Query(sql) + result, err := structureQuery(ctx, sql) if err != nil { + if *errp == nil { + *errp = err + } return counts } for _, row := range result.Rows { @@ -479,10 +506,14 @@ func structureDepth2(ctx *ExecContext, modules []structureModule) error { structureWorkflows(ctx, m.Name, wfByModule[m.Name], false) // Pages (from catalog) - structurePages(ctx, m.Name) + if err := structurePages(ctx, m.Name); err != nil { + return err + } // Snippets (from catalog) - structureSnippets(ctx, m.Name) + if err := structureSnippets(ctx, m.Name); err != nil { + return err + } // Java Actions outputJavaActions(ctx, m.Name, jaByModule[m.Name], false) @@ -512,13 +543,19 @@ func structureDepth2(ctx *ExecContext, modules []structureModule) error { } // OData Clients - structureODataClients(ctx, m.Name) + if err := structureODataClients(ctx, m.Name); err != nil { + return err + } // OData Services - structureODataServices(ctx, m.Name) + if err := structureODataServices(ctx, m.Name); err != nil { + return err + } // Business Event Services - structureBusinessEventServices(ctx, m.Name) + if err := structureBusinessEventServices(ctx, m.Name); err != nil { + return err + } } return nil @@ -649,10 +686,14 @@ func structureDepth3(ctx *ExecContext, modules []structureModule) error { structureWorkflows(ctx, m.Name, wfByModule[m.Name], true) // Pages - structurePages(ctx, m.Name) + if err := structurePages(ctx, m.Name); err != nil { + return err + } // Snippets - structureSnippets(ctx, m.Name) + if err := structureSnippets(ctx, m.Name); err != nil { + return err + } // Java Actions (with param names) outputJavaActions(ctx, m.Name, jaByModule[m.Name], true) @@ -686,11 +727,17 @@ func structureDepth3(ctx *ExecContext, modules []structureModule) error { } // OData - structureODataClients(ctx, m.Name) - structureODataServices(ctx, m.Name) + if err := structureODataClients(ctx, m.Name); err != nil { + return err + } + if err := structureODataServices(ctx, m.Name); err != nil { + return err + } // Business Event Services - structureBusinessEventServices(ctx, m.Name) + if err := structureBusinessEventServices(ctx, m.Name); err != nil { + return err + } } return nil @@ -772,68 +819,97 @@ func structureEntities(ctx *ExecContext, moduleName string, dm *domainmodel.Doma } } -// structurePages outputs pages for a module from the catalog. -func structurePages(ctx *ExecContext, moduleName string) { - // Query pages from catalog - result, err := ctx.Catalog.Query(fmt.Sprintf( +// structurePages outputs pages for a module from the catalog, each annotated +// with its outermost data widgets — `Page M.P [DataView, Datagrid]`. +// +// "Outermost" means no data widget above it in the page's widget tree: those are +// the widgets that establish a data context. A data widget inside a layout grid +// or container still counts (they are not data contexts), and one nested inside +// another data widget does not (it is a detail of its parent). Filtering on the +// page root instead would drop nearly everything, since real pages wrap their +// content in a layout grid. +// +// widgets_data is only filled by a full catalog build, so on a fast-mode catalog +// every page prints bare. +func structurePages(ctx *ExecContext, moduleName string) error { + result, err := structureQuery(ctx, fmt.Sprintf( "select Name from pages where ModuleName = '%s' ORDER by Name", escapeSQLString(moduleName))) - if err != nil || len(result.Rows) == 0 { - return + if err != nil { + return err + } + if len(result.Rows) == 0 { + return nil } - // Try to get top-level data widgets from widgets table - widgetsByPage := make(map[string][]string) - widgetResult, err := ctx.Catalog.Query(fmt.Sprintf( - "select ContainerQualifiedName, WidgetType, EntityRef from widgets where ModuleName = '%s' and ParentWidget = '' ORDER by ContainerQualifiedName, WidgetType", + widgetResult, err := structureQuery(ctx, fmt.Sprintf( + "select Id, ParentWidgetId, ContainerQualifiedName, WidgetType, EntityRef from widgets "+ + "where ModuleName = '%s' and ContainerType = 'PAGE' ORDER by ContainerQualifiedName, WidgetType, Id", escapeSQLString(moduleName))) - if err == nil { - for _, row := range widgetResult.Rows { - pageName := asString(row[0]) - widgetType := asString(row[1]) - entityRef := asString(row[2]) - - // Only include data-bound widgets - if !isDataWidget(widgetType) { - continue + if err != nil { + return err + } + type pageWidget struct { + parent, page, widgetType, entity string + } + byID := make(map[string]pageWidget, len(widgetResult.Rows)) + var order []string + for _, row := range widgetResult.Rows { + id := asString(row[0]) + byID[id] = pageWidget{parent: asString(row[1]), page: asString(row[2]), widgetType: asString(row[3]), entity: asString(row[4])} + order = append(order, id) + } + hasDataAncestor := func(w pageWidget) bool { + // ParentWidgetId chains end at the page root (""); the visited set only + // guards against a malformed cycle. + seen := map[string]bool{} + for p := w.parent; p != "" && !seen[p]; p = byID[p].parent { + seen[p] = true + if isDataWidget(byID[p].widgetType) { + return true } + } + return false + } - // Extract short widget type name - shortType := shortWidgetType(widgetType) - if entityRef != "" { - // Extract entity name from qualified name - shortEntity := shortName(entityRef) - widgetsByPage[pageName] = append(widgetsByPage[pageName], fmt.Sprintf("%s<%s>", shortType, shortEntity)) - } else { - widgetsByPage[pageName] = append(widgetsByPage[pageName], shortType) - } + widgetsByPage := make(map[string][]string) + for _, id := range order { + w := byID[id] + if !isDataWidget(w.widgetType) || hasDataAncestor(w) { + continue + } + label := shortWidgetType(w.widgetType) + if w.entity != "" { + label += "<" + shortName(w.entity) + ">" } + widgetsByPage[w.page] = append(widgetsByPage[w.page], label) } for _, row := range result.Rows { - name := asString(row[0]) - qualName := moduleName + "." + name - if widgets, ok := widgetsByPage[qualName]; ok && len(widgets) > 0 { + qualName := moduleName + "." + asString(row[0]) + if widgets := widgetsByPage[qualName]; len(widgets) > 0 { fmt.Fprintf(ctx.Output, " Page %s [%s]\n", qualName, strings.Join(widgets, ", ")) } else { fmt.Fprintf(ctx.Output, " Page %s\n", qualName) } } + return nil } // structureSnippets outputs snippets for a module from the catalog. -func structureSnippets(ctx *ExecContext, moduleName string) { - result, err := ctx.Catalog.Query(fmt.Sprintf( +func structureSnippets(ctx *ExecContext, moduleName string) error { + result, err := structureQuery(ctx, fmt.Sprintf( "select Name from snippets where ModuleName = '%s' ORDER by Name", escapeSQLString(moduleName))) - if err != nil || len(result.Rows) == 0 { - return + if err != nil { + return err } for _, row := range result.Rows { name := asString(row[0]) fmt.Fprintf(ctx.Output, " Snippet %s.%s\n", moduleName, name) } + return nil } // outputJavaActions outputs java actions for a module. @@ -898,12 +974,12 @@ func formatJATypeDisplay(typeStr string) string { } // structureODataClients outputs OData clients for a module. -func structureODataClients(ctx *ExecContext, moduleName string) { - result, err := ctx.Catalog.Query(fmt.Sprintf( +func structureODataClients(ctx *ExecContext, moduleName string) error { + result, err := structureQuery(ctx, fmt.Sprintf( "select Name, ODataVersion from odata_clients where ModuleName = '%s' ORDER by Name", escapeSQLString(moduleName))) - if err != nil || len(result.Rows) == 0 { - return + if err != nil { + return err } for _, row := range result.Rows { @@ -916,15 +992,16 @@ func structureODataClients(ctx *ExecContext, moduleName string) { fmt.Fprintf(ctx.Output, " ODataClient %s\n", qualName) } } + return nil } // structureODataServices outputs OData services for a module. -func structureODataServices(ctx *ExecContext, moduleName string) { - result, err := ctx.Catalog.Query(fmt.Sprintf( +func structureODataServices(ctx *ExecContext, moduleName string) error { + result, err := structureQuery(ctx, fmt.Sprintf( "select Name, Path, EntitySetCount from odata_services where ModuleName = '%s' ORDER by Name", escapeSQLString(moduleName))) - if err != nil || len(result.Rows) == 0 { - return + if err != nil { + return err } for _, row := range result.Rows { @@ -938,15 +1015,16 @@ func structureODataServices(ctx *ExecContext, moduleName string) { fmt.Fprintf(ctx.Output, " ODataService %s\n", qualName) } } + return nil } // structureBusinessEventServices outputs business event services for a module. -func structureBusinessEventServices(ctx *ExecContext, moduleName string) { - result, err := ctx.Catalog.Query(fmt.Sprintf( +func structureBusinessEventServices(ctx *ExecContext, moduleName string) error { + result, err := structureQuery(ctx, fmt.Sprintf( "select Name, MessageCount, PublishCount, SubscribeCount from business_event_services where ModuleName = '%s' ORDER by Name", escapeSQLString(moduleName))) - if err != nil || len(result.Rows) == 0 { - return + if err != nil { + return err } for _, row := range result.Rows { @@ -973,6 +1051,7 @@ func structureBusinessEventServices(ctx *ExecContext, moduleName string) { fmt.Fprintf(ctx.Output, " BusinessEventService %s\n", qualName) } } + return nil } // structureWorkflows outputs workflows for a module. @@ -1131,7 +1210,8 @@ func shortName(qualifiedName string) string { func shortWidgetType(widgetType string) string { // Widget types may look like "DataGrid", "DataView", "ListView", etc. // Or pluggable widgets like "com.mendix.widget.web.datagrid2.DataGrid2" - if idx := strings.LastIndex(widgetType, "."); idx >= 0 { + // Built-in widgets are stored as "Forms$DataView", "Forms$ListView", … + if idx := strings.LastIndexAny(widgetType, ".$"); idx >= 0 { return widgetType[idx+1:] } return widgetType diff --git a/mdl/executor/cmd_structure_page_widgets_test.go b/mdl/executor/cmd_structure_page_widgets_test.go new file mode 100644 index 0000000000..f21342f44c --- /dev/null +++ b/mdl/executor/cmd_structure_page_widgets_test.go @@ -0,0 +1,153 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "github.com/mendixlabs/mxcli/sdk/microflows" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/catalog" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// pageWidgetsCatalog runs the REAL catalog builder, in full mode (the only mode +// that fills widgets_data), over one page whose stored widget tree has the three +// shapes the structure annotation has to tell apart: +// +// LayoutGrid — not a data widget +// └ column → Data grid 2 (Order) — a data widget, but not at the page root +// DataView (Customer) — a data widget at the root +// └ ListView (Order) — nested inside another data widget +// +// Seeding widgets_data by hand would only prove the reader agrees with the +// columns the test typed; the reader named a column (ParentWidget) that the +// builder has never written. +func pageWidgetsCatalog(t *testing.T) *catalog.Catalog { + t.Helper() + const mod = model.ID("mod-sales") + entity := func(name string) map[string]any { + return map[string]any{ + "$Type": "Forms$DataViewSource", + "EntityRef": map[string]any{"$Type": "DomainModels$DirectEntityRef", "Entity": name}, + } + } + page := map[string]any{ + "$Type": "Forms$Page", + "FormCall": map[string]any{ + "$Type": "Forms$LayoutCall", + "Arguments": []any{int32(2), map[string]any{ + "$Type": "Forms$FormCallArgument", + "Widgets": []any{int32(2), + map[string]any{ + "$ID": "w-grid", "$Type": "Forms$LayoutGrid", "Name": "layoutGrid1", + "Rows": []any{int32(2), map[string]any{ + "$Type": "Forms$LayoutGridRow", + "Columns": []any{int32(2), map[string]any{ + "$Type": "Forms$LayoutGridColumn", + "Widgets": []any{int32(2), map[string]any{ + "$ID": "w-dg2", "$Type": "CustomWidgets$CustomWidget", "Name": "dataGrid1", + "Type": map[string]any{"$Type": "CustomWidgets$CustomWidgetType", "WidgetId": "com.mendix.widget.web.datagrid.Datagrid"}, + "Object": map[string]any{"$Type": "CustomWidgets$WidgetObject", "Properties": []any{int32(2), entity("Sales.Order")}}, + }}, + }}, + }}, + }, + map[string]any{ + "$ID": "w-dv", "$Type": "Forms$DataView", "Name": "dataView1", + "DataSource": entity("Sales.Customer"), + "Widgets": []any{int32(2), map[string]any{ + "$ID": "w-lv", "$Type": "Forms$ListView", "Name": "listView1", + "DataSource": entity("Sales.Order"), + }}, + }, + }, + }}, + }, + } + be := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + GetProjectSettingsFunc: func() (*model.ProjectSettings, error) { return &model.ProjectSettings{}, nil }, + ListModuleSettingsFunc: func() ([]*types.ModuleSettings, error) { return nil, nil }, + ListModulesFunc: func() ([]*model.Module, error) { + return []*model.Module{{BaseElement: model.BaseElement{ID: mod}, Name: "Sales"}}, nil + }, + ListPagesFunc: func() ([]*pages.Page, error) { + return []*pages.Page{ + {BaseElement: model.BaseElement{ID: "pg-1"}, ContainerID: mod, Name: "Order_Overview"}, + {BaseElement: model.BaseElement{ID: "pg-2"}, ContainerID: mod, Name: "Home"}, + }, nil + }, + ListNanoflowsFunc: func() ([]*microflows.Nanoflow, error) { return nil, nil }, + ListRulesFunc: func() ([]*microflows.Rule, error) { return nil, nil }, + GetNavigationFunc: func() (*types.NavigationDocument, error) { return &types.NavigationDocument{}, nil }, + GetRawUnitFunc: func(id model.ID) (map[string]any, error) { + if id == "pg-1" { + return page, nil + } + return nil, nil + }, + } + cat, err := catalog.New() + if err != nil { + t.Fatalf("catalog.New: %v", err) + } + t.Cleanup(func() { cat.Close() }) + b := catalog.NewBuilder(cat, be) + b.SetFullMode(true) + if err := b.Build(nil); err != nil { + t.Fatalf("catalog build: %v", err) + } + return cat +} + +// `show structure depth 2|3` annotates each page with its data widgets, e.g. +// `Page Sales.Order_Overview [DataView, ...]`. It never did: the query +// filtered on a ParentWidget column that widgets_data does not have, the error +// was discarded, and every page printed bare. +// +// What the annotation lists is the page's OUTERMOST data widgets — the ones +// that establish a data context — so a data grid inside a layout grid counts +// (a layout grid is not a data context) and a list view nested in a data view +// does not (it is a detail of that data view). +func TestStructurePagesAnnotatesOutermostDataWidgets(t *testing.T) { + cat := pageWidgetsCatalog(t) + ctx, buf := newMockCtx(t) + ctx.Catalog = cat + + if err := structurePages(ctx, "Sales"); err != nil { + t.Fatalf("structurePages: %v", err) + } + out := buf.String() + want := " Page Sales.Order_Overview [DataView, Datagrid]\n" + if !strings.Contains(out, want) { + t.Errorf("page annotation missing or wrong.\nwant line: %q\ngot:\n%s", want, out) + } + if !strings.Contains(out, " Page Sales.Home\n") { + t.Errorf("a page without data widgets should print bare; got:\n%s", out) + } +} + +// The structure queries are fixed SQL against tables the builder owns, so a +// failing one is a column or table drift inside mxcli — never a property of +// the user's project. Discarding the error is how ParentWidget went unnoticed +// from the initial commit on; it has to reach the caller. +func TestStructurePagesReportsCatalogQueryErrors(t *testing.T) { + cat := pageWidgetsCatalog(t) + if _, err := cat.Query("DROP VIEW widgets"); err != nil { + t.Fatalf("drop view: %v", err) + } + ctx, _ := newMockCtx(t) + ctx.Catalog = cat + + err := structurePages(ctx, "Sales") + if err == nil { + t.Fatal("structurePages swallowed a failing catalog query") + } + if !strings.Contains(err.Error(), "widgets") { + t.Errorf("error should name the failing query; got: %v", err) + } +} diff --git a/mdl/executor/issue1328_excluded_activity_test.go b/mdl/executor/issue1328_excluded_activity_test.go new file mode 100644 index 0000000000..35a1547a5a --- /dev/null +++ b/mdl/executor/issue1328_excluded_activity_test.go @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "reflect" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// mendixlabs/mxcli#1328: "@excluded on microflow activities does not seem to +// work anymore" — `@excluded declare $Variable Boolean = false;` was written +// with the activity enabled. +func TestExcludedActivityIsDisabled_Issue1328(t *testing.T) { + for _, tc := range []struct{ name, body string }{ + {"declare", "@excluded declare $Variable Boolean = false; return;"}, + {"log", "@excluded log info node 'x' 'hi'; return;"}, + } { + t.Run(tc.name, func(t *testing.T) { + prog, errs := visitor.Build("create or modify microflow M.F () begin " + tc.body + " end;") + for _, e := range errs { + t.Fatalf("parse error: %v", e) + } + mf := prog.Statements[0].(*ast.CreateMicroflowStmt) + fb := &flowBuilder{ + posX: 100, posY: 100, spacing: HorizontalSpacing, + varTypes: map[string]string{}, declaredVars: map[string]string{}, + } + fb.buildFlowGraph(mf.Body, nil) + var acts []*microflows.ActionActivity + for _, obj := range fb.objects { + if a, ok := obj.(*microflows.ActionActivity); ok { + acts = append(acts, a) + } + } + if len(acts) != 1 { + t.Fatalf("want 1 activity, got %d", len(acts)) + } + if !acts[0].Disabled { + t.Errorf("activity written enabled: @excluded was dropped") + } + }) + } +} + +// mergeStatementAnnotations copies ActivityAnnotations field by field, and +// #1328 was one field it skipped. Every field set on a statement must survive +// the merge unless it is deliberately consumed elsewhere (listed below), so a +// field added later fails here instead of being dropped in silence. +func TestMergeStatementAnnotationsCopiesEveryField(t *testing.T) { + notMerged := map[string]string{ + "Start": "read off the first statement directly (buildFlowGraph)", + "InvalidCurves": "validation only", + "InvalidAnchors": "validation only", + "InvalidNotes": "validation only", + "UnknownNames": "validation only", + } + pos := &ast.Position{X: 1, Y: 2} + anchors := &ast.FlowAnchors{From: ast.AnchorSideRight, To: ast.AnchorSideLeft} + full := &ast.ActivityAnnotations{ + Position: pos, Caption: "c", Color: "Green", + Notes: []ast.MicroflowAnnotation{{Text: "n"}}, + FreeNotes: []ast.MicroflowAnnotation{{Text: "f"}}, + Excluded: true, Anchor: anchors, + TrueBranchAnchor: anchors, FalseBranchAnchor: anchors, + IteratorAnchor: anchors, BodyTailAnchor: anchors, + Curve: &ast.FlowCurve{From: pos, To: pos}, Merge: pos, Start: pos, + InvalidCurves: []string{"x"}, InvalidAnchors: []string{"x"}, + InvalidNotes: []string{"x"}, UnknownNames: []string{"x"}, + } + fb := &flowBuilder{} + fb.mergeStatementAnnotations(&ast.LogStmt{Annotations: full}) + + src, got := reflect.ValueOf(full).Elem(), reflect.ValueOf(fb.pendingAnnotations).Elem() + for i := 0; i < src.NumField(); i++ { + name := src.Type().Field(i).Name + if _, skip := notMerged[name]; skip { + continue + } + if src.Field(i).IsZero() { + t.Fatalf("test fixture leaves %s zero — set it so the merge is exercised", name) + } + if !reflect.DeepEqual(src.Field(i).Interface(), got.Field(i).Interface()) { + t.Errorf("mergeStatementAnnotations dropped %s", name) + } + } +} diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index 54ec83fcf9..809a15cca1 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -701,6 +701,9 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext sc.warnExcluded("microflow", s.Name.String(), refErrors) refErrors = nil } + // The signature's types, which exec resolves (and refuses) whether or + // not the document is excluded — so these are never relaxed. + refErrors = append(flowSignatureErrors(ctx, sc, "microflow", s.Name, s.Parameters, s.ReturnType), refErrors...) if len(validationErrors) > 0 || len(refErrors) > 0 { return flowValidationError("microflow", s.Name.String(), validationErrors, refErrors) } @@ -719,6 +722,10 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext return mdlerrors.NewValidationf("rule '%s' has validation errors:\n - %s", s.Name.String(), strings.Join(validationErrors, "\n - ")) } + if sigErrors := flowSignatureErrors(ctx, sc, "rule", s.Name, s.Parameters, s.ReturnType); len(sigErrors) > 0 { + return mdlerrors.NewValidationf("rule '%s' has reference errors:\n - %s", + s.Name.String(), strings.Join(sigErrors, "\n - ")) + } if refErrors := validateFlowBodyReferences(ctx, s.Body, sc); len(refErrors) > 0 { if s.Excluded { sc.warnExcluded("rule", s.Name.String(), refErrors) @@ -744,6 +751,9 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext sc.warnExcluded("nanoflow", s.Name.String(), refErrors) refErrors = nil } + // The signature's types, which exec resolves (and refuses) whether or + // not the document is excluded — so these are never relaxed. + refErrors = append(flowSignatureErrors(ctx, sc, "nanoflow", s.Name, s.Parameters, s.ReturnType), refErrors...) if len(validationErrors) > 0 || len(refErrors) > 0 { return flowValidationError("nanoflow", s.Name.String(), validationErrors, refErrors) } @@ -753,6 +763,12 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext return mdlerrors.NewNotFound("module", s.Name.Module) } } + // The parameters' entities: exec resolves each and stops on one it + // cannot find, excluded page or not. + if paramErrors := documentParameterErrors(ctx, sc, "page", s.Name, s.Parameters); len(paramErrors) > 0 { + return mdlerrors.NewValidationf("page '%s' has reference errors:\n - %s", + s.Name.String(), strings.Join(paramErrors, "\n - ")) + } // Every widget-bearing field, not just the bare body — see pageWidgets. pageWidgets := allPageWidgets(s) // Validate widget references (DataSource, Action, Snippet). An EXCLUDED @@ -804,6 +820,10 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext return mdlerrors.NewNotFound("module", s.Name.Module) } } + if paramErrors := documentParameterErrors(ctx, sc, "snippet", s.Name, s.Parameters); len(paramErrors) > 0 { + return mdlerrors.NewValidationf("snippet '%s' has reference errors:\n - %s", + s.Name.String(), strings.Join(paramErrors, "\n - ")) + } // Validate widget references (DataSource, Action, Snippet). A snippet // has no @excluded of its own; one exec keeps excluded (the carry) is // treated as an excluded page is. @@ -846,6 +866,16 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext return mdlerrors.NewValidationf("workflow '%s' has reference errors:\n - %s", s.Name.String(), strings.Join(refErrors, "\n - ")) } + case *ast.CreatePublishedRestServiceStmt: + return validatePublishedRestAuthMicroflow(ctx, s.Authentication, sc) + case *ast.AlterPublishedRestServiceStmt: + for _, a := range s.Actions { + if set, ok := a.(*ast.PublishedRestSetAction); ok { + if err := validatePublishedRestAuthMicroflow(ctx, set.Authentication, sc); err != nil { + return err + } + } + } case *ast.AlterWorkflowStmt: if refErrors := validateAlterWorkflowRefs(ctx, s, sc); len(refErrors) > 0 { return mdlerrors.NewValidationf("workflow '%s' has reference errors:\n - %s", @@ -2009,3 +2039,23 @@ func flowValidationError(kind, name string, validationErrors, refErrors []string } return mdlerrors.NewValidationf("%s", strings.Join(parts, "\n ")) } + +// validatePublishedRestAuthMicroflow is exec's MDL-REST04 at check time: the +// custom-authentication microflow exists, and a project microflow has a +// signature Mendix accepts. A microflow the script creates is known by name +// only here; exec checks its signature once it exists. +func validatePublishedRestAuthMicroflow(ctx *ExecContext, auth *ast.PublishedRestAuthentication, sc *scriptContext) error { + if auth == nil || auth.Microflow == "" { + return nil + } + if mf, ok := liveMicroflowsByQualifiedName(ctx)[auth.Microflow]; ok { + if problem := publishedRestAuthMicroflowProblem(mf); problem != "" { + return mdlerrors.NewValidationf("MDL-REST04: authentication microflow %s %s", auth.Microflow, problem) + } + return nil + } + if sc.microflows[auth.Microflow] { + return nil + } + return mdlerrors.NewValidationf("MDL-REST04: authentication microflow not found: %s", auth.Microflow) +} diff --git a/mdl/executor/validate_commit_in_loop.go b/mdl/executor/validate_commit_in_loop.go index d699db433e..5452853c70 100644 --- a/mdl/executor/validate_commit_in_loop.go +++ b/mdl/executor/validate_commit_in_loop.go @@ -10,7 +10,8 @@ import ( ) // checkCommitInLoop (MDL-PERF01) reports a commit inside a loop, which is one -// database round trip per iteration. +// database round trip per iteration — a `commit` statement, or a `create` / +// `change` with a commit clause (mendixlabs/mxcli#1217). // // `lint` has known this as CONV011 since long before this rule, and that is the // problem it exists to solve rather than duplicate: CONV011 reads the STORED @@ -50,6 +51,22 @@ func (v *microflowValidator) checkCommitInLoop(body []ast.MicroflowStatement) { "per iteration (`lint` reports this as CONV011)", st.Variable), fmt.Sprintf("Add $%s to a list inside the loop and commit the list once after it", st.Variable)) } + case *ast.ChangeObjectStmt: + // mendixlabs/mxcli#1217: the commit clause is the same round + // trip per iteration as a separate commit, and CONV011 counts it. + if inLoop && st.Commit != ast.CommitNo { + v.addViolation("MDL-PERF01", linter.SeverityWarning, + fmt.Sprintf("change of $%s commits inside a loop, so it runs one database round trip "+ + "per iteration (`lint` reports this as CONV011)", st.Variable), + fmt.Sprintf("Drop the commit clause, add $%s to a list inside the loop and commit the list once after it", st.Variable)) + } + case *ast.CreateObjectStmt: + if inLoop && st.Commit != ast.CommitNo { + v.addViolation("MDL-PERF01", linter.SeverityWarning, + fmt.Sprintf("create of $%s commits inside a loop, so it runs one database round trip "+ + "per iteration (`lint` reports this as CONV011)", st.Variable), + fmt.Sprintf("Drop the commit clause, add $%s to a list inside the loop and commit the list once after it", st.Variable)) + } case *ast.LoopStmt: walk(st.Body, true) case *ast.WhileStmt: diff --git a/mdl/executor/validate_commit_in_loop_test.go b/mdl/executor/validate_commit_in_loop_test.go index b307262645..c67130d369 100644 --- a/mdl/executor/validate_commit_in_loop_test.go +++ b/mdl/executor/validate_commit_in_loop_test.go @@ -110,3 +110,42 @@ func TestLoopWithoutCommitIsNotReported(t *testing.T) { t.Errorf("a loop with no commit was flagged: %v", got) } } + +// mendixlabs/mxcli#1217: `change $T (...) commit;` and `$X = create ... commit;` +// in a loop commit once per iteration exactly as a separate `commit $T;` does. +// CONV011 now counts them, so this rule must too — the boundary is CONV011's. +func TestCommitClauseOnChangeOrCreateInLoopIsReported(t *testing.T) { + got := commitInLoopViolations(t, []ast.MicroflowStatement{ + loopOver("Items", + &ast.ChangeObjectStmt{Variable: "Item", Commit: ast.CommitYes}, + &ast.CreateObjectStmt{Variable: "Log", Commit: ast.CommitYesWithoutEvents}, + &ast.IfStmt{ThenBody: []ast.MicroflowStatement{ + &ast.ChangeObjectStmt{Variable: "Other", Commit: ast.CommitYes}, + }}, + ), + }) + if len(got) != 3 { + t.Fatalf("got %d findings, want 3 (change commit / create commit / change commit in branch): %v", len(got), got) + } + for _, msg := range got { + if !strings.Contains(msg, "CONV011") { + t.Errorf("message does not name CONV011:\n%s", msg) + } + } +} + +// CONTROL: the commit clause outside a loop, and a non-committing change inside +// one, are not per-iteration round trips. +func TestCommitClauseOutsideLoopOrAbsentIsNotReported(t *testing.T) { + got := commitInLoopViolations(t, []ast.MicroflowStatement{ + &ast.ChangeObjectStmt{Variable: "Item", Commit: ast.CommitYes}, + &ast.CreateObjectStmt{Variable: "Log", Commit: ast.CommitYes}, + loopOver("Items", + &ast.ChangeObjectStmt{Variable: "Item", Commit: ast.CommitNo}, + &ast.CreateObjectStmt{Variable: "Log"}, + ), + }) + if len(got) != 0 { + t.Errorf("flagged a commit clause that is not per iteration: %v", got) + } +} diff --git a/mdl/executor/validate_declared_types.go b/mdl/executor/validate_declared_types.go new file mode 100644 index 0000000000..9640e0153e --- /dev/null +++ b/mdl/executor/validate_declared_types.go @@ -0,0 +1,146 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// declaredTypeResolver answers "does this type name exist" for the types a +// document DECLARES — a flow's parameters and return type, a page's or +// snippet's parameters — as opposed to the ones its body uses. +// +// The body was resolved (retrieve, create, a data source), and so were an +// association's endpoints (#555) and an EXTENDS target (#972); the signature +// never was. `create microflow M.F ($p: System.Nope)` passed +// `check --references`, as did `M.Nope` and a page parameter of either. exec +// refuses the user-module shapes and every page parameter after the statements +// before them are written; a System entity in a flow signature it writes by +// name (isBuiltinModuleEntity), leaving a reference nothing resolves. +// +// System is resolved like any module: the backend lists its virtual domain +// model and its enumerations, so System.User resolves and System.Nope does not. +// The sets are built once per statement, lazily — most signatures are primitive. +type declaredTypeResolver struct { + ctx *ExecContext + sc *scriptContext + entities map[string]bool + // listsSystem is whether the backend lists the System module at all. + listsSystem bool +} + +func newDeclaredTypeResolver(ctx *ExecContext, sc *scriptContext) *declaredTypeResolver { + return &declaredTypeResolver{ctx: ctx, sc: sc} +} + +func (r *declaredTypeResolver) entityExists(qn ast.QualifiedName) bool { + if r.sc.producesEntity(qn) { + return true + } + if r.entities == nil { + r.entities = buildEntityQualifiedNames(r.ctx) + for name := range r.entities { + if strings.HasPrefix(name, "System.") { + r.listsSystem = true + break + } + } + } + if qn.Module == "System" && !r.listsSystem { + // A backend that does not expose the System module cannot say a + // System entity is missing — absence of the whole module is not + // evidence about one name in it. + return true + } + return r.entities[qn.String()] +} + +func (r *declaredTypeResolver) enumExists(qn ast.QualifiedName) bool { + return (r.sc != nil && r.sc.enumerations[qn.String()]) || enumerationExists(r.ctx, qn.String()) +} + +// check returns the problem with one declared type, or "" when it resolves or +// names nothing. where says whose type it is ("parameter $p of microflow M.F"). +// +// A bare `Module.Name` is ambiguous — the parser cannot tell an entity from an +// enumeration and records either kind — so it resolves if either exists, as +// exec's own fallback does. Only the explicit `Enumeration(…)` / `enum …` +// spelling is held to enumerations alone. +func (r *declaredTypeResolver) check(dt ast.DataType, where string) string { + var qn *ast.QualifiedName + enumOnly := false + switch { + case dt.EntityRef != nil: + qn = dt.EntityRef + case dt.Kind == ast.TypeEnumeration && dt.EnumRef != nil: + qn = dt.EnumRef + enumOnly = dt.ExplicitEnum + } + if qn == nil || qn.Module == "" || qn.Name == "" { + return "" + } + if enumOnly { + if r.enumExists(*qn) { + return "" + } + return fmt.Sprintf("%s: enumeration %s does not exist", where, qn.String()) + } + if r.entityExists(*qn) || r.enumExists(*qn) { + return "" + } + msg := fmt.Sprintf("%s: entity %s does not exist", where, qn.String()) + if hint := entityNameHint(*qn); hint != "" { + msg += ". " + hint + } + return msg +} + +// flowSignatureErrors resolves a microflow's, nanoflow's or rule's parameter +// and return types. +func flowSignatureErrors(ctx *ExecContext, sc *scriptContext, kind string, name ast.QualifiedName, + params []ast.MicroflowParam, ret *ast.MicroflowReturnType) []string { + if !ctx.Connected() { + return nil + } + r := newDeclaredTypeResolver(ctx, sc) + var errs []string + for _, p := range params { + if msg := r.check(p.Type, fmt.Sprintf("parameter $%s of %s %s", p.Name, kind, name.String())); msg != "" { + errs = append(errs, msg) + } + } + if ret != nil { + if msg := r.check(ret.Type, fmt.Sprintf("return type of %s %s", kind, name.String())); msg != "" { + errs = append(errs, msg) + } + } + return errs +} + +// documentParameterErrors resolves a page's or snippet's entity parameters, +// the ones exec resolves through pageBuilder.resolveEntity (and refuses). +func documentParameterErrors(ctx *ExecContext, sc *scriptContext, kind string, name ast.QualifiedName, + params []ast.PageParameter) []string { + if !ctx.Connected() { + return nil + } + r := newDeclaredTypeResolver(ctx, sc) + var errs []string + for _, p := range params { + if pageParamBSONType(p.Type) != "" || p.EntityType.Name == "" || p.EntityType.Module == "" { + continue // primitive, or a bare name MDL's own checks report + } + if r.entityExists(p.EntityType) { + continue + } + msg := fmt.Sprintf("parameter $%s of %s %s: entity %s does not exist", p.Name, kind, name.String(), p.EntityType.String()) + if hint := entityNameHint(p.EntityType); hint != "" { + msg += ". " + hint + } + errs = append(errs, msg) + } + return errs +} diff --git a/mdl/executor/validate_microflow.go b/mdl/executor/validate_microflow.go index 9263a38b52..f976bca0f3 100644 --- a/mdl/executor/validate_microflow.go +++ b/mdl/executor/validate_microflow.go @@ -577,6 +577,8 @@ func (v *microflowValidator) checkExprFunctions(label string, expr ast.Expressio suggestion = fmt.Sprintf( "'%s' is an aggregate activity, not an expression function. Assign it to a variable first: $n = %s($List); then use $n in the expression.", u.Name, u.Name) + } else if u.Token != "" { + suggestion = fmt.Sprintf("Use the %s token instead of '%s()'.", u.Token, u.Name) } else { suggestion = "Use a built-in Mendix expression function (see 'mxcli syntax microflow')." if u.Suggestion != "" { diff --git a/mdl/executor/validate_microflow_expr_test.go b/mdl/executor/validate_microflow_expr_test.go index 84a35080c2..6ea2023e3d 100644 --- a/mdl/executor/validate_microflow_expr_test.go +++ b/mdl/executor/validate_microflow_expr_test.go @@ -69,6 +69,13 @@ func TestValidateMicroflow_UnknownFunction(t *testing.T) { {"unknown in log message", "log info 'device: ' + currentDeviceType();", true, ""}, {"unknown in log template param", "log info 'device: {1}' with ({1} = currentDeviceType());", true, ""}, {"known func in log message", "log info 'x: ' + toUpperCase($x);", false, ""}, + // currentDateTime() was listed in funcTable, so check and exec passed it and + // the build failed: CE0117 on 11.14.0 in a microflow and a nanoflow alike. + // The current time is the [%CurrentDateTime%] token, which builds clean — + // the hint names it rather than a did-you-mean on spelling. + {"currentDateTime() is not a Mendix built-in", "declare $d DateTime = currentDateTime();", true, "[%CurrentDateTime%]"}, + {"currentDateTime() nested in a call", "declare $d DateTime = addDays(currentDateTime(), 1);", true, "[%CurrentDateTime%]"}, + {"the [%CurrentDateTime%] token is accepted", "declare $d DateTime = addDays([%CurrentDateTime%], 1);", false, ""}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/mdl/executor/validate_nanoflow_test.go b/mdl/executor/validate_nanoflow_test.go index bfe3442b5e..ce77717e95 100644 --- a/mdl/executor/validate_nanoflow_test.go +++ b/mdl/executor/validate_nanoflow_test.go @@ -56,6 +56,17 @@ begin if $x != empty then set $y = trunc(1.5); end if; +end;`, + wantMDL: true, + }, + { + // currentDateTime() is CE0117 in a nanoflow too (11.14.0); the + // [%CurrentDateTime%] token below is the working spelling. + name: "currentDateTime() is not a built-in", + src: `create nanoflow Test.NF_Now () returns DateTime +begin + declare $d DateTime = currentDateTime(); + return $d; end;`, wantMDL: true, }, diff --git a/mdl/executor/validate_page_button_context.go b/mdl/executor/validate_page_button_context.go index d8ea51bc60..63482465dc 100644 --- a/mdl/executor/validate_page_button_context.go +++ b/mdl/executor/validate_page_button_context.go @@ -56,13 +56,11 @@ func checkButtonContextTree(widgets []*ast.WidgetV3, controlBarOf string, inCont if w == nil { continue } - if a := w.GetAction(); a != nil { - if controlBarOf != "" && !inContext { - out = append(out, checkControlBarAction(a, w.Name, controlBarOf, locationPrefix)...) - } - if nearest != "" { - out = append(out, checkOwnContainerName(a, w.Name, nearest, locationPrefix)...) - } + if a := w.GetAction(); a != nil && controlBarOf != "" && !inContext { + out = append(out, checkControlBarAction(a, w.Name, controlBarOf, locationPrefix)...) + } + if nearest != "" { + out = append(out, checkOwnContainerName(w, nearest, locationPrefix)...) } childContext := inContext || isObjectContextContainer(w) childNearest := nearest @@ -80,33 +78,59 @@ func checkButtonContextTree(widgets []*ast.WidgetV3, controlBarOf string, inCont return out } -// checkOwnContainerName flags an action argument (and its chained THEN action) -// that reads the nearest data container by its widget name — `$dvGate` from a -// button directly inside dvGate. A container's name is a variable only for the -// containers nested BELOW it; in its own context the object is -// $currentObject, and mxbuild reports the name as CE0117 "Error(s) in -// expression." (mendixlabs/mxcli#1324, measured on 11.14.0 for data views, -// list views, galleries and data grids alike). -func checkOwnContainerName(a *ast.ActionV3, widgetName, nearest, locationPrefix string) []linter.Violation { +// ownNameExprProps are the expression-valued widget properties evaluated in +// the widget's enclosing context, keyed as the visitor stores them (`Visible:` +// and `Editable:` expressions land under the *If keys) and named as written. +var ownNameExprProps = []struct{ key, written string }{ + {"VisibleIf", "Visible"}, {"EditableIf", "Editable"}, {"DynamicClasses", "DynamicClasses"}, +} + +// checkOwnContainerName flags a widget that reads the nearest data container +// by its widget name — `$dvGate` from a widget directly inside dvGate. A +// container's name is a variable only for the containers nested BELOW it; in +// its own context the object is $currentObject (mendixlabs/mxcli#1324). +// +// Every slot here is evaluated in the widget's enclosing context, and each was +// measured on mxbuild 11.14.0 against a control one data view deeper: action +// arguments (and a chained THEN action's), a data source's flow arguments, +// Visible, Editable and DynamicClasses are CE0117 "Error(s) in expression.", +// and a data source's XPath `where` is CE0161 "Error(s) in XPath constraint.". +func checkOwnContainerName(w *ast.WidgetV3, nearest, locationPrefix string) []linter.Violation { var out []linter.Violation - for a != nil { + flag := func(slot, expr, code string) { + if !exprReadsVariable(expr, nearest) { + return + } + out = append(out, linter.Violation{ + RuleID: "MDL-BUTTON02", + Severity: linter.SeverityError, + Message: fmt.Sprintf( + "%s: `%s` reads `%s` in its %s, but `%s` is the data container the widget sits in directly — its name is a variable only inside a data container nested below it (%s)", + locationPrefix, w.Name, expr, slot, nearest, code), + Suggestion: fmt.Sprintf( + "Use $currentObject for the object of `%s` here (`$currentObject/Attr` for an attribute); `$%s` works from a data view, list or grid nested inside it.", + nearest, nearest), + }) + } + for a := w.GetAction(); a != nil; a = a.ThenAction { for _, arg := range a.Args { - s, ok := arg.Value.(string) - if !ok || !exprReadsVariable(s, nearest) { - continue + if v, ok := arg.Value.(string); ok { + flag(a.Type+" action argument", v, "CE0117") } - out = append(out, linter.Violation{ - RuleID: "MDL-BUTTON02", - Severity: linter.SeverityError, - Message: fmt.Sprintf( - "%s: `%s` passes `%s` to its %s action, but `%s` is the data container the widget sits in directly — its name is a variable only inside a data container nested below it (CE0117)", - locationPrefix, widgetName, s, a.Type, nearest), - Suggestion: fmt.Sprintf( - "Use $currentObject for the object of `%s` here (`$currentObject/Attr` for an attribute); `$%s` works from a data view, list or grid nested inside it.", - nearest, nearest), - }) } - a = a.ThenAction + } + if ds := w.GetDataSource(); ds != nil { + for _, arg := range ds.Args { + if v, ok := arg.Value.(string); ok { + flag("data source argument", v, "CE0117") + } + } + flag("data source XPath", ds.Where, "CE0161") + } + for _, p := range ownNameExprProps { + if v, ok := w.Properties[p.key].(string); ok { + flag(p.written+" expression", v, "CE0117") + } } return out } diff --git a/mdl/executor/validate_page_own_container_name_test.go b/mdl/executor/validate_page_own_container_name_test.go index d2f0629570..b922252017 100644 --- a/mdl/executor/validate_page_own_container_name_test.go +++ b/mdl/executor/validate_page_own_container_name_test.go @@ -116,3 +116,48 @@ func TestValidatePageButtonContext_OwnContainerName_Suggestion(t *testing.T) { t.Errorf("message should name CE0117 and suggest $currentObject: %+v", vs[0]) } } + +// The same scope rule holds outside actions. Measured with `mx check` 11.14.0 +// (each slot once directly in dvP, once one data view deeper as the control): +// a nested data view's microflow data-source argument, Visible, Editable and +// DynamicClasses are CE0117 and a nested list's XPath `where` is CE0161 when +// they read the container they sit in directly; every control builds clean. +func TestValidatePageButtonContext_OwnContainerName_OutsideActions(t *testing.T) { + src := `create page G52.Probe3 (Title: 'Probe3', Layout: Atlas_Core.PopupLayout, Params: ( $Gate: G52.Gate )) { + dataview dvP (DataSource: $Gate) { + dataview dvDsOwn (DataSource: microflow G52.DS_Gate(Gate = $dvP)) { + dynamictext dtA (Content: 'a') + } + container cVisOwn (Visible: $dvP/Name != '') { + dynamictext dtB (Content: 'b') + } + textbox tbEdOwn (Attribute: Name, Editable: $dvP/Name != '') + container cClsOwn (DynamicClasses: if $dvP/Name = '' then 'a' else 'b') { + dynamictext dtE (Content: 'e') + } + listview lvXpOwn (DataSource: database from G52.Gate where [Name = $dvP/Name]) { + dynamictext dtF (Content: 'f') + } + dataview dvMid (DataSource: $Gate) { + dataview dvDsOuter (DataSource: microflow G52.DS_Gate(Gate = $dvP)) { + dynamictext dtC (Content: 'c') + } + container cVisOuter (Visible: $dvP/Name != '') { + dynamictext dtD (Content: 'd') + } + textbox tbEdOuter (Attribute: Name, Editable: $dvP/Name != '') + container cClsOuter (DynamicClasses: if $dvP/Name = '' then 'a' else 'b') { + dynamictext dtG (Content: 'g') + } + listview lvXpOuter (DataSource: database from G52.Gate where [Name = $dvP/Name]) { + dynamictext dtH (Content: 'h') + } + } + } +};` + got := strings.Join(ownContainerFlagged(t, src), ",") + want := "cClsOwn,cVisOwn,dvDsOwn,lvXpOwn,tbEdOwn" + if got != want { + t.Fatalf("flagged %q, want %q (mx check 11.14.0's CE0117/CE0161 set)", got, want) + } +} diff --git a/mdl/exprcheck/func_checker.go b/mdl/exprcheck/func_checker.go index d8eec30d82..6466404157 100644 --- a/mdl/exprcheck/func_checker.go +++ b/mdl/exprcheck/func_checker.go @@ -72,9 +72,14 @@ var funcTable = map[string]funcSig{ "urlDecode": {args: []TypeKind{KindString}, ret: KindString}, // Type conversion - "toString": {args: []TypeKind{KindAny}, ret: KindString}, - "parseInteger": {args: []TypeKind{KindString}, ret: KindInteger}, - "parseDecimal": {args: []TypeKind{KindString}, ret: KindDecimal}, + "toString": {args: []TypeKind{KindAny}, ret: KindString}, + // parseInteger(value [, default]) and parseDecimal(value [, format [, default]]) + // return the default when the text does not parse; a two-argument + // parseDecimal takes either a format or a default. parseBoolean has no + // default overload: `parseBoolean($s, false)` is CE0117. All measured with + // mx check on 11.14.0 (mendixlabs/mxcli#1216). + "parseInteger": {args: []TypeKind{KindString, KindInteger}, minArgs: 1, ret: KindInteger}, + "parseDecimal": {args: []TypeKind{KindString, KindString, KindDecimal}, minArgs: 1, ret: KindDecimal}, "parseBoolean": {args: []TypeKind{KindString}, ret: KindBoolean}, // formatDecimal(value [, format [, languageTag]]) — format is optional "formatDecimal": {args: []TypeKind{KindDecimal, KindString, KindString}, minArgs: 1, ret: KindString}, @@ -113,8 +118,8 @@ var funcTable = map[string]funcSig{ "isSynced": {args: []TypeKind{KindAny}, ret: KindBoolean}, "isSyncing": {args: []TypeKind{KindAny}, ret: KindBoolean}, - // DateTime — construction - "currentDateTime": {args: []TypeKind{}, ret: KindDateTime}, + // DateTime — construction. There is no currentDateTime(): the current time + // is the [%CurrentDateTime%] token (see tokenFuncs in unknown_funcs.go). // dateTime/dateTimeUTC(year, month, day [, hour, minute, second]) — 3 or 6 args "dateTime": {args: []TypeKind{KindInteger, KindInteger, KindInteger, KindInteger, KindInteger, KindInteger}, minArgs: 3, ret: KindDateTime}, "dateTimeUTC": {args: []TypeKind{KindInteger, KindInteger, KindInteger, KindInteger, KindInteger, KindInteger}, minArgs: 3, ret: KindDateTime}, @@ -199,13 +204,16 @@ var funcTable = map[string]funcSig{ "formatDateTime": {args: []TypeKind{KindDateTime, KindString}, ret: KindString}, "formatTime": {args: []TypeKind{KindDateTime, KindString}, minArgs: 1, ret: KindString}, "formatDate": {args: []TypeKind{KindDateTime, KindString}, minArgs: 1, ret: KindString}, - "parseDateTime": {args: []TypeKind{KindString, KindString}, ret: KindDateTime}, + // parseDateTime/parseDateTimeUTC(value, format [, default]) — the default is + // returned when the text does not parse (mendixlabs/mxcli#1216; 3 args build + // at 0 errors on 11.14.0, 4 args are CE0117). + "parseDateTime": {args: []TypeKind{KindString, KindString, KindDateTime}, minArgs: 2, ret: KindDateTime}, // DateTime — formatting / parsing (UTC calendar) "formatDateTimeUTC": {args: []TypeKind{KindDateTime, KindString}, ret: KindString}, "formatTimeUTC": {args: []TypeKind{KindDateTime, KindString}, minArgs: 1, ret: KindString}, "formatDateUTC": {args: []TypeKind{KindDateTime, KindString}, minArgs: 1, ret: KindString}, - "parseDateTimeUTC": {args: []TypeKind{KindString, KindString}, ret: KindDateTime}, + "parseDateTimeUTC": {args: []TypeKind{KindString, KindString, KindDateTime}, minArgs: 2, ret: KindDateTime}, // DateTime — epoch conversion "dateTimeToEpoch": {args: []TypeKind{KindDateTime}, ret: KindLong}, diff --git a/mdl/exprcheck/parse_default_arity_test.go b/mdl/exprcheck/parse_default_arity_test.go new file mode 100644 index 0000000000..695630ca62 --- /dev/null +++ b/mdl/exprcheck/parse_default_arity_test.go @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: Apache-2.0 + +package exprcheck + +import "testing" + +// mendixlabs/mxcli#1216: the parse functions take an optional default value, +// returned when the text does not parse. `check` rejected it — "parseDateTimeUTC() +// expects 2 argument(s), got 3. [E006]" — while mx check on 11.14.0 reports 0 +// errors. Each row below was built against mx check 11.14.0; the "bad" rows are +// CE0117 there, so the fix must not widen past them. +func TestFuncChecker_ParseDefaultValue_Arity(t *testing.T) { + ok := []string{ + "parseDateTime($T, 'yyyy-MM-dd')", + "parseDateTime($T, 'yyyy-MM-dd', empty)", + "parseDateTime($T, 'yyyy-MM-dd', $Fallback)", + "parseDateTimeUTC($T, 'yyyy-MM-dd')", + "parseDateTimeUTC($T, 'yyyy-MM-dd', empty)", + "parseDateTimeUTC($T, 'yyyy-MM-dd', $Fallback)", + "parseInteger($T)", + "parseInteger($T, 0)", + "parseDecimal($T)", + "parseDecimal($T, 0)", + "parseDecimal($T, '#.##')", + "parseDecimal($T, '#.##', 0)", + } + for _, src := range ok { + _, hs := NewParser().Parse(src, Context{Microflow: "M.F"}) + if hasCode(hs, "E006") { + t.Errorf("%s must not fire E006: %+v", src, hs) + } + } + + bad := []string{ + "parseDateTime($T)", + "parseDateTimeUTC($T)", + "parseDateTime($T, 'yyyy-MM-dd', empty, empty)", + "parseDateTimeUTC($T, 'yyyy-MM-dd', empty, empty)", + "parseInteger($T, 0, 0)", + "parseDecimal($T, '#.##', 0, 0)", + "parseBoolean($T, false)", + } + for _, src := range bad { + _, hs := NewParser().Parse(src, Context{Microflow: "M.F"}) + if !hasCode(hs, "E006") { + t.Errorf("%s must fire E006, got %+v", src, hs) + } + } +} diff --git a/mdl/exprcheck/unknown_funcs.go b/mdl/exprcheck/unknown_funcs.go index 6f15568946..c98df7e00c 100644 --- a/mdl/exprcheck/unknown_funcs.go +++ b/mdl/exprcheck/unknown_funcs.go @@ -11,6 +11,16 @@ type FuncRef struct { Line int Column int Suggestion string // nearest known built-in, or "" if none is close + Token string // the [%Token%] the call stands in for, or "" (see tokenFuncs) +} + +// tokenFuncs maps a call that reads like a built-in but is a Mendix token +// (lower-cased name) to that token. A did-you-mean on spelling cannot reach it, +// since the right answer is not a function. currentDateTime() was in funcTable +// and failed the build with CE0117 in a microflow and a nanoflow on 11.14.0, +// where [%CurrentDateTime%] builds at 0 errors. Add a row only when measured. +var tokenFuncs = map[string]string{ + "currentdatetime": "[%CurrentDateTime%]", } // UnknownFunctionCalls parses a Mendix expression and returns every call to a @@ -42,6 +52,7 @@ func UnknownFunctionCalls(src string) []FuncRef { Line: c.Pos().Line, Column: c.Pos().Column, Suggestion: nearestFunc(c.Name), + Token: tokenFuncs[strings.ToLower(c.Name)], }) }) return out diff --git a/mdl/grammar/MDLParser.g4 b/mdl/grammar/MDLParser.g4 index 5213098b81..bb3f469a45 100644 --- a/mdl/grammar/MDLParser.g4 +++ b/mdl/grammar/MDLParser.g4 @@ -284,8 +284,11 @@ alterStatement | alterModuleJarDepStatement ; +// `set ( Key: value, … )` takes create's property list (R3); the +// `set Key = 'value'` form is the older spelling for the string properties. alterPublishedRestServiceAction - : SET publishedRestAlterAssignment (COMMA publishedRestAlterAssignment)* + : SET LPAREN publishedRestProperty (COMMA publishedRestProperty)* COMMA? RPAREN + | SET publishedRestAlterAssignment (COMMA publishedRestAlterAssignment)* | ADD publishedRestResource | DROP RESOURCE STRING_LITERAL ; diff --git a/mdl/grammar/domains/MDLService.g4 b/mdl/grammar/domains/MDLService.g4 index 28c147606e..0bf9eaf2ed 100644 --- a/mdl/grammar/domains/MDLService.g4 +++ b/mdl/grammar/domains/MDLService.g4 @@ -174,8 +174,20 @@ createPublishedRestServiceStatement LBRACE publishedRestResource* RBRACE ; +// `Authentication: none | ( method, … )` is the one non-string value: the +// methods in the order written, which is the order Studio Pro stores them +// (mendixlabs/mxcli#1331). The visitor keys on the shape, not the key, so a +// string `Authentication: 'basic'` is reported as misshapen, not dropped. publishedRestProperty : identifierOrKeyword COLON STRING_LITERAL + | identifierOrKeyword COLON NONE + | identifierOrKeyword COLON LPAREN publishedRestAuthMethod (COMMA publishedRestAuthMethod)* COMMA? RPAREN + ; + +publishedRestAuthMethod + : BASIC + | SESSION + | MICROFLOW qualifiedName ; publishedRestResource diff --git a/mdl/linter/rules/conv_loop_commit.go b/mdl/linter/rules/conv_loop_commit.go index a4796068de..6f9eba0eb0 100644 --- a/mdl/linter/rules/conv_loop_commit.go +++ b/mdl/linter/rules/conv_loop_commit.go @@ -21,7 +21,7 @@ func (r *NoCommitInLoopRule) Category() string { return "perform func (r *NoCommitInLoopRule) DefaultSeverity() linter.Severity { return linter.SeverityWarning } func (r *NoCommitInLoopRule) Description() string { - return "Commit actions should not be inside loops (N+1 performance issue)" + return "Commits should not be inside loops, whether a commit action or a create/change with commit (N+1 performance issue)" } func (r *NoCommitInLoopRule) Check(ctx *linter.LintContext) []linter.Violation { @@ -55,13 +55,13 @@ func findCommitsInLoops(objects []microflows.MicroflowObject, mf linter.Microflo if !insideLoop || act.Action == nil { continue } - if _, ok := act.Action.(*microflows.CommitObjectsAction); ok { + if what := committingActionKind(act.Action); what != "" { *violations = append(*violations, linter.Violation{ RuleID: r.ID(), Severity: r.DefaultSeverity(), - Message: fmt.Sprintf("%s '%s.%s' has a Commit action inside a loop. "+ + Message: fmt.Sprintf("%s '%s.%s' has %s inside a loop. "+ "This causes N+1 database operations.", - mf.DocumentNounTitle(), mf.ModuleName, mf.Name), + mf.DocumentNounTitle(), mf.ModuleName, mf.Name, what), Location: linter.Location{ Module: mf.ModuleName, DocumentType: mf.DocumentNoun(), @@ -78,3 +78,28 @@ func findCommitsInLoops(objects []microflows.MicroflowObject, mf linter.Microflo } } } + +// committingActionKind describes an action that commits to the database, or +// returns "" for one that does not. A create or change activity with Commit set +// is one round trip per iteration exactly as a separate commit is — Studio Pro's +// recommender flags both as MXP004 (mendixlabs/mxcli#1217). Any value other than +// No commits: YesWithoutEvents skips the event handlers, not the database. +func committingActionKind(action microflows.MicroflowAction) string { + switch a := action.(type) { + case *microflows.CommitObjectsAction: + return "a Commit action" + case *microflows.ChangeObjectAction: + if commits(a.Commit) { + return "a Change object action with Commit enabled" + } + case *microflows.CreateObjectAction: + if commits(a.Commit) { + return "a Create object action with Commit enabled" + } + } + return "" +} + +func commits(c microflows.CommitType) bool { + return c != "" && c != microflows.CommitTypeNo +} diff --git a/mdl/linter/rules/conv_loop_commit_test.go b/mdl/linter/rules/conv_loop_commit_test.go index 44c1de8e3d..bf0bd0f0e5 100644 --- a/mdl/linter/rules/conv_loop_commit_test.go +++ b/mdl/linter/rules/conv_loop_commit_test.go @@ -89,3 +89,56 @@ func TestNoCommitInLoopRule_Metadata(t *testing.T) { t.Errorf("Category = %q, want performance", r.Category()) } } + +func loopWithAction(action microflows.MicroflowAction) []microflows.MicroflowObject { + return []microflows.MicroflowObject{ + µflows.LoopedActivity{ + ObjectCollection: µflows.MicroflowObjectCollection{ + Objects: []microflows.MicroflowObject{ + µflows.ActionActivity{Action: action}, + }, + }, + }, + } +} + +// mendixlabs/mxcli#1217: `change $T (...) commit;` inside a loop is the same +// round trip per iteration as a separate `commit $T;`, and Studio Pro's +// recommender flags both (MXP004). Only the separate commit was reported. +func TestFindCommitsInLoops_CommitOnChangeOrCreateInsideLoop(t *testing.T) { + cases := []struct { + name string + action microflows.MicroflowAction + want int + }{ + {"change commit", µflows.ChangeObjectAction{Commit: microflows.CommitTypeYes}, 1}, + {"change commit without events", µflows.ChangeObjectAction{Commit: microflows.CommitTypeYesWithoutEvents}, 1}, + {"create commit", µflows.CreateObjectAction{Commit: microflows.CommitTypeYes}, 1}, + {"create commit without events", µflows.CreateObjectAction{Commit: microflows.CommitTypeYesWithoutEvents}, 1}, + // CONTROLS: no commit on the activity is no round trip. + {"change no commit", µflows.ChangeObjectAction{Commit: microflows.CommitTypeNo}, 0}, + {"create no commit", µflows.CreateObjectAction{Commit: microflows.CommitTypeNo}, 0}, + {"change commit unset", µflows.ChangeObjectAction{}, 0}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var violations []linter.Violation + findCommitsInLoops(loopWithAction(tc.action), testMicroflow(), NewNoCommitInLoopRule(), &violations, false) + if len(violations) != tc.want { + t.Fatalf("got %d violations, want %d: %v", len(violations), tc.want, violations) + } + }) + } +} + +// CONTROL: the same committing change outside any loop is one round trip. +func TestFindCommitsInLoops_CommitOnChangeOutsideLoop(t *testing.T) { + objects := []microflows.MicroflowObject{ + µflows.ActionActivity{Action: µflows.ChangeObjectAction{Commit: microflows.CommitTypeYes}}, + } + var violations []linter.Violation + findCommitsInLoops(objects, testMicroflow(), NewNoCommitInLoopRule(), &violations, false) + if len(violations) != 0 { + t.Errorf("expected 0 violations outside loop, got %d", len(violations)) + } +} diff --git a/mdl/visitor/visitor_rest.go b/mdl/visitor/visitor_rest.go index cb6ce6d538..63daa81520 100644 --- a/mdl/visitor/visitor_rest.go +++ b/mdl/visitor/visitor_rest.go @@ -3,6 +3,7 @@ package visitor import ( + "fmt" "strconv" "strings" @@ -362,12 +363,13 @@ func (b *Builder) ExitCreatePublishedRestServiceStatement(ctx *parser.CreatePubl } } - // Parse properties (Path, Version, ServiceName) + // Parse properties (Path, Version, ServiceName, Authentication) for i, propCtx := range ctx.AllPublishedRestProperty() { pc := propCtx.(*parser.PublishedRestPropertyContext) - key := identifierOrKeywordText(pc.IdentifierOrKeyword().(*parser.IdentifierOrKeywordContext)) - b.checkProperty(pc, &publishedRestSchema, key, shapeString) - val := unquoteStringLit(pc.STRING_LITERAL()) + key, val, auth, ok := b.publishedRestProperty(pc) + if !ok { + continue + } switch strings.ToLower(key) { case "path": stmt.Path = val @@ -375,6 +377,8 @@ func (b *Builder) ExitCreatePublishedRestServiceStatement(ctx *parser.CreatePubl stmt.Version = val case "servicename": stmt.ServiceName = val + case "authentication": + stmt.Authentication = auth case "folder": if !folderClause { stmt.Folder = val @@ -394,6 +398,68 @@ func (b *Builder) ExitCreatePublishedRestServiceStatement(ctx *parser.CreatePubl b.statements = append(b.statements, stmt) } +// publishedRestProperty reads one `Key: value` of a published REST service's +// property list: the key, the string value, or the authentication value when +// the property is `Authentication`. The value's shape is checked against the +// key, so `Authentication: 'basic'` or `Path: none` is reported rather than +// read as empty. ok is false when error recovery left the property without a +// key. +func (b *Builder) publishedRestProperty(pc *parser.PublishedRestPropertyContext) (key, val string, auth *ast.PublishedRestAuthentication, ok bool) { + ik, isIK := pc.IdentifierOrKeyword().(*parser.IdentifierOrKeywordContext) + if !isIK || ik == nil { + return "", "", nil, false + } + key = identifierOrKeywordText(ik) + shape := shapeOther + switch { + case pc.STRING_LITERAL() != nil: + shape = shapeString + val = unquoteStringLit(pc.STRING_LITERAL()) + case pc.NONE() != nil: + shape = shapeNone + auth = &ast.PublishedRestAuthentication{} + case pc.LPAREN() != nil: + shape = shapeAuthMethods + auth = b.publishedRestAuthMethods(pc) + } + b.checkProperty(pc, &publishedRestSchema, key, shape) + if shape == shapeOther { + // Error recovery: the syntax error is already recorded. + return "", "", nil, false + } + return key, val, auth, true +} + +// publishedRestAuthMethods reads `( basic, session, microflow M.F )` in the +// order written, which is the order Studio Pro stores the methods in. +func (b *Builder) publishedRestAuthMethods(pc *parser.PublishedRestPropertyContext) *ast.PublishedRestAuthentication { + auth := &ast.PublishedRestAuthentication{} + seen := map[string]bool{} + for _, mCtx := range pc.AllPublishedRestAuthMethod() { + mc := mCtx.(*parser.PublishedRestAuthMethodContext) + var method string + switch { + case mc.BASIC() != nil: + method = "Basic" + case mc.SESSION() != nil: + method = "Session" + case mc.MICROFLOW() != nil && mc.QualifiedName() != nil: + method = "Microflow" + auth.Microflow = buildQualifiedName(mc.QualifiedName()).String() + default: + continue + } + if seen[method] { + b.addError(fmt.Errorf("line %d: authentication method '%s' listed twice", + mc.GetStart().GetLine(), strings.ToLower(method))) + continue + } + seen[method] = true + auth.Methods = append(auth.Methods, method) + } + return auth +} + // buildPublishedRestResourceDef converts a PublishedRestResourceContext to a // PublishedRestResourceDef AST node. Shared by CREATE and ALTER. // Returns nil when the resource name token is absent (ANTLR error-recovery path). @@ -475,6 +541,24 @@ func (b *Builder) exitAlterPublishedRestServiceStatement(ctx *parser.AlterStatem continue } + // SET ( Key: value, ... ) — create's property list (R3) + if ac.SET() != nil && ac.LPAREN() != nil { + set := &ast.PublishedRestSetAction{Changes: make(map[string]string)} + for _, propCtx := range ac.AllPublishedRestProperty() { + key, val, auth, ok := b.publishedRestProperty(propCtx.(*parser.PublishedRestPropertyContext)) + if !ok { + continue + } + if strings.EqualFold(key, "authentication") { + set.Authentication = auth + continue + } + set.Changes[key] = val + } + stmt.Actions = append(stmt.Actions, set) + continue + } + // SET key = 'value' [, ...] if ac.SET() != nil { changes := make(map[string]string) diff --git a/mdl/visitor/visitor_rest_auth_test.go b/mdl/visitor/visitor_rest_auth_test.go new file mode 100644 index 0000000000..77aafcd0fb --- /dev/null +++ b/mdl/visitor/visitor_rest_auth_test.go @@ -0,0 +1,96 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "reflect" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +func publishedRestWith(prop string) string { + return `create or modify published rest service M.Api ( + Path: 'rest/api/v1', + ` + prop + ` +) { + resource 'items' { get '' microflow M.GetItems; } +};` +} + +// The methods keep the order written: Studio Pro stores them in the order they +// were ticked (ako/TestApp: `Microflow, Session`), so sorting would rewrite an +// unchanged service. +func TestPublishedRestAuthentication_Methods(t *testing.T) { + cases := []struct { + prop string + want *ast.PublishedRestAuthentication + }{ + {"Authentication: none", &ast.PublishedRestAuthentication{}}, + {"Authentication: (basic)", &ast.PublishedRestAuthentication{Methods: []string{"Basic"}}}, + {"Authentication: (microflow M.Auth, session)", &ast.PublishedRestAuthentication{Methods: []string{"Microflow", "Session"}, Microflow: "M.Auth"}}, + {"Authentication: (basic, session, microflow M.Auth,)", &ast.PublishedRestAuthentication{Methods: []string{"Basic", "Session", "Microflow"}, Microflow: "M.Auth"}}, + } + for _, c := range cases { + t.Run(c.prop, func(t *testing.T) { + prog, errs := Build("mdl 1;\n" + publishedRestWith(c.prop)) + if len(errs) > 0 { + t.Fatalf("errors: %v", errs) + } + stmt := prog.Statements[len(prog.Statements)-1].(*ast.CreatePublishedRestServiceStmt) + if !reflect.DeepEqual(stmt.Authentication, c.want) { + t.Errorf("Authentication = %+v, want %+v", stmt.Authentication, c.want) + } + }) + } +} + +// Not stating it is not `none`: the executor keeps the stored setting. +func TestPublishedRestAuthentication_OmittedIsNil(t *testing.T) { + prog, errs := Build(publishedRestWith("Version: '1.0.0'")) + if len(errs) > 0 { + t.Fatalf("errors: %v", errs) + } + if a := prog.Statements[0].(*ast.CreatePublishedRestServiceStmt).Authentication; a != nil { + t.Errorf("Authentication = %+v, want nil", a) + } +} + +func TestPublishedRestAuthentication_Rejections(t *testing.T) { + cases := map[string]string{ + "Authentication: (basic, basic)": "listed twice", + "Authentication: (microflow M.A, microflow M.B)": "listed twice", + "Authentication: 'basic'": "takes", + "Path: none": "takes", + "Version: (basic)": "takes", + } + for prop, want := range cases { + t.Run(prop, func(t *testing.T) { + _, errs := Build("mdl 1;\n" + publishedRestWith(prop)) + var all []string + for _, e := range errs { + all = append(all, e.Error()) + } + if !strings.Contains(strings.Join(all, "\n"), want) { + t.Errorf("errors %q do not mention %q", all, want) + } + }) + } +} + +func TestAlterPublishedRestService_SetPropertyList(t *testing.T) { + prog, errs := Build(`alter published rest service M.Api set (Version: '2.0.0', Authentication: (session));`) + if len(errs) > 0 { + t.Fatalf("errors: %v", errs) + } + stmt := prog.Statements[0].(*ast.AlterPublishedRestServiceStmt) + set := stmt.Actions[0].(*ast.PublishedRestSetAction) + if set.Changes["Version"] != "2.0.0" { + t.Errorf("Changes = %v", set.Changes) + } + want := &ast.PublishedRestAuthentication{Methods: []string{"Session"}} + if !reflect.DeepEqual(set.Authentication, want) { + t.Errorf("Authentication = %+v, want %+v", set.Authentication, want) + } +} diff --git a/mdl/visitor/visitor_rest_test.go b/mdl/visitor/visitor_rest_test.go index df35c308dd..d73608eddc 100644 --- a/mdl/visitor/visitor_rest_test.go +++ b/mdl/visitor/visitor_rest_test.go @@ -100,3 +100,40 @@ func TestCreatePublishedRestService_EndResourceSyntax_NoPanic(t *testing.T) { prog, _ := Build(input) _ = prog } + +// TestCreatePublishedRestService_NonStringPropertyValue_NoPanic reproduces +// mendixlabs/mxcli#1331: a property value that is not a string literal +// (`Authentication: microflow M.F`, `Authentication: Basic`, +// `Folder: microflow M.F`) crashed the binary with a nil pointer dereference in +// unquoteStringLit. Build walks a failed parse on purpose, so the rule context +// exists with no STRING_LITERAL child; the author must get the syntax error. +func TestCreatePublishedRestService_NonStringPropertyValue_NoPanic(t *testing.T) { + for name, value := range map[string]string{ + "microflow": "Authentication: microflow MyFirstModule.AuthMf", + "keyword": "Authentication: Basic", + "folder-non-str": "Folder: microflow MyFirstModule.AuthMf", + } { + t.Run(name, func(t *testing.T) { + input := `CREATE OR MODIFY PUBLISHED REST SERVICE MyFirstModule.TestApi ( + Path: 'rest/test/v1', + Version: '1.0.0', + ServiceName: 'Test API', + ` + value + ` +) +{ + RESOURCE 'items' { + GET '' MICROFLOW MyFirstModule.GetItems; + } +};` + defer func() { + if r := recover(); r != nil { + t.Fatalf("panic for %q: %v", value, r) + } + }() + _, errs := Build(input) + if len(errs) == 0 { + t.Fatalf("expected a syntax error for %q, got none", value) + } + }) + } +} diff --git a/mdl/visitor/visitor_strict_properties.go b/mdl/visitor/visitor_strict_properties.go index 2c5d42162d..7af31f8d09 100644 --- a/mdl/visitor/visitor_strict_properties.go +++ b/mdl/visitor/visitor_strict_properties.go @@ -43,29 +43,31 @@ var misshapedPropertyValue = langver.Change{ type propShape string const ( - shapeString propShape = "a string ('…')" - shapeDollar propShape = "a $$…$$ block" - shapeName propShape = "a name" - shapeQName propShape = "a qualified name (Module.Name)" - shapeNumber propShape = "a number" - shapeBool propShape = "true or false" - shapeVarList propShape = `a variable list ("Key": String, …)` - shapeVariable propShape = "a $variable" - shapeConstant propShape = "a constant (@Module.Name)" - shapeNone propShape = "none" - shapeBasic propShape = "basic (Username: …, Password: …)" - shapeMethod propShape = "an HTTP method (get, post, put, patch, delete)" - shapeParamList propShape = "a parameter list ($name: Type, …)" - shapeHeaderList propShape = "a header list ('Name' = value, …)" - shapeTemplate propShape = "template '…'" - shapeMapping propShape = "mapping Module.Entity { … }" - shapeJSONFrom propShape = "json from $var" - shapeFileFrom propShape = "file from $var" - shapeJSONAs propShape = "json as $var" - shapeStringAs propShape = "string as $var" - shapeFileAs propShape = "file as $var" - shapeStatusAs propShape = "status as $var" - shapeOther propShape = "some other value" + shapeString propShape = "a string ('…')" + shapeDollar propShape = "a $$…$$ block" + shapeName propShape = "a name" + shapeQName propShape = "a qualified name (Module.Name)" + shapeNumber propShape = "a number" + shapeBool propShape = "true or false" + shapeVarList propShape = `a variable list ("Key": String, …)` + shapeVariable propShape = "a $variable" + shapeConstant propShape = "a constant (@Module.Name)" + shapeNone propShape = "none" + shapeBasic propShape = "basic (Username: …, Password: …)" + // shapeAuthMethods is a published REST service's authentication methods. + shapeAuthMethods propShape = "a method list (basic, session, microflow Module.Name)" + shapeMethod propShape = "an HTTP method (get, post, put, patch, delete)" + shapeParamList propShape = "a parameter list ($name: Type, …)" + shapeHeaderList propShape = "a header list ('Name' = value, …)" + shapeTemplate propShape = "template '…'" + shapeMapping propShape = "mapping Module.Entity { … }" + shapeJSONFrom propShape = "json from $var" + shapeFileFrom propShape = "file from $var" + shapeJSONAs propShape = "json as $var" + shapeStringAs propShape = "string as $var" + shapeFileAs propShape = "file as $var" + shapeStatusAs propShape = "status as $var" + shapeOther propShape = "some other value" ) // propKey is one key a property list reads, and the shapes it takes. @@ -182,6 +184,7 @@ var publishedRestSchema = propSchema{on: "a published REST service", keys: []pro {"Path", []propShape{shapeString}}, {"Version", []propShape{shapeString}}, {"ServiceName", []propShape{shapeString}}, + {"Authentication", []propShape{shapeNone, shapeAuthMethods}}, {"Folder", []propShape{shapeString}}, }} diff --git a/mdl/visitor/visitor_string_escapes.go b/mdl/visitor/visitor_string_escapes.go index 0a4ead7808..a2cfe550ff 100644 --- a/mdl/visitor/visitor_string_escapes.go +++ b/mdl/visitor/visitor_string_escapes.go @@ -44,7 +44,15 @@ func newScriptStream(input string, implicit langver.Version) antlr.CharStream { // unquoteStringLit is the value of a STRING_LITERAL (a terminal node, or a // rule whose text is one), read under the escape rule it was lexed with. +// +// A nil node reads as "": Build walks a failed parse on purpose, so a rule that +// requires a STRING_LITERAL can still arrive here without one (an unquoted +// value under error recovery, mendixlabs/mxcli#1331). The listener has already +// recorded the syntax error, and that is what the author must see — not a panic. func unquoteStringLit(n interface{ GetText() string }) string { + if n == nil { + return "" + } text := n.GetText() if !lexedWithStrictEscapes(n) { return unquoteString(text) diff --git a/model/types.go b/model/types.go index 73e51b4d89..05e2bc3331 100644 --- a/model/types.go +++ b/model/types.go @@ -789,6 +789,14 @@ type PublishedRestService struct { Excluded bool `json:"excluded,omitempty"` AllowedRoles []string `json:"allowedRoles,omitempty"` Resources []*PublishedRestResource `json:"resources,omitempty"` + + // AuthenticationTypes are the methods a caller may authenticate with, in + // the order stored: "Basic", "Session", "Microflow". Empty is "Requires + // authentication = No". AuthenticationMicroflow is the custom + // authentication microflow's qualified name, "" unless "Microflow" is + // among the methods (Studio Pro clears it when Custom is unticked). + AuthenticationTypes []string `json:"authenticationTypes,omitempty"` + AuthenticationMicroflow string `json:"authenticationMicroflow,omitempty"` } // GetName returns the service's name.