From 4f8eb993a6602871be99a108969c6457f9e8b353 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Go=C5=82embiewski?= Date: Sat, 26 Sep 2026 16:55:19 +0200 Subject: [PATCH] fix(rest): derive published REST operation parameters from the microflow create published rest service wrote only the path's {name} placeholders as operation parameters, each as a String. Any other microflow parameter failed mx check with CE0350, and an {id} bound to an Integer failed CE6539. MDL has no syntax for operation parameters, so a query parameter, a body or an upload could not be built; a generated app parsed HttpRequest/Uri by hand instead. Studio Pro derives them from the microflow: a parameter named in the path is a path parameter, an object or a list is the body, System.HttpRequest and HttpResponse are left out, and anything else is a query parameter. create and alter now do the same before the service is written, each parameter with the microflow parameter's own type. When the microflow does not exist yet, the operation keeps its path parameters and exec says so. Measured on Mendix 11.12.1 with mdl-examples/bug-tests/published-rest-operation-parameters.mdl: 4 mx check errors on v0.24.0 and on main, 0 with this change. At runtime GET .../status?orderNumber=ORD%2F2026%2F0012&count=3 answers ORD/2026/0012:3, and re-executing describe output leaves the service unchanged. Refs mendixlabs/mxcli#1206 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + CHANGELOG.md | 1 + cmd/mxcli/syntax/features_integration.go | 3 +- docs-site/src/examples/rest-integration.md | 17 +- .../published-rest-operation-parameters.mdl | 60 +++++++ .../modelsdk/published_rest_params_test.go | 116 ++++++++++++ mdl/backend/modelsdk/published_rest_write.go | 66 ++++--- mdl/executor/cmd_published_rest.go | 59 ++++++ .../cmd_published_rest_params_test.go | 169 ++++++++++++++++++ model/types.go | 34 ++++ 10 files changed, 500 insertions(+), 26 deletions(-) create mode 100644 mdl-examples/bug-tests/published-rest-operation-parameters.mdl create mode 100644 mdl/backend/modelsdk/published_rest_params_test.go create mode 100644 mdl/executor/cmd_published_rest_params_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 8bb82beaac..1d722e4734 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -713,3 +713,4 @@ {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`textbox t (Attribute: FullName)` at the top of a page (CREATE PAGE/SNIPPET, a plain container, or ALTER PAGE … INSERT at page level) passed plain `mxcli check`, `exec --no-check`/ALTER reported success, and `bson dump` showed `AttributeRef: null` — mxbuild 11.13.0: CE0544 \"This widget can only function inside a data context\" + CE7005 (textbox/textarea/datepicker/checkbox/radiobuttons/dropdown), CE0402 (dynamictext Attribute:), CE0642 (combobox). Qualified `Mod.Ent.Attr` there is stored and fails CE0544/CE2421/CE1365/CE7247 \"Move this widget into a data container\" + CE7006. `Attribute: $P/Attr` / `$currentObject/Attr` dropped even INSIDE a data view.", "cause": "resolveAttributePath returns the bare name when entityContext is \"\", and attributeRefToGen (and widgetobj setAttributeRefField) write nil for any path with < 2 dots, so the binding vanished between builder and writer; refuseBareAttributeRefs never sees it because no Attribute string is emitted. The only refusal (validatePageContextTree) runs in the --references phase for CREATE PAGE/SNIPPET, so plain check, --no-check and ALTER were unguarded. `$x/Attr` parses via the generic property rule as an *ast.DataSourceV3, so GetAttribute() returns \"\" and every builder skipped it.", "file": "mdl/executor/cmd_pages_input_binding_context.go (inputBindingProblem, checkInputBinding, validateInputBindingContext = MDL-WIDGET34), wired in cmd_pages_builder_v3_widgets.go (6 input builders + buildDynamicTextV3), widget_engine.go (primary Attribute mapping), validate_widgets.go (validateWidgetTreeIn); tests cmd_pages_input_binding_context_test.go; bug-tests input-binding-without-context{,.fail}.mdl", "insight": "Reuse the MDL-PAGEARG01 three-state context (pageArgContext known/present) rather than entityContext==\"\" as the 'outside a data container' signal: entityContext is also empty INSIDE a container whose flow cannot be resolved (excluded ShareFeedback_Logo), where DESCRIBE writes qualified names that must keep building — refusing qualified-on-empty-entity would have broken that round trip. So known-absent context refuses bare AND qualified; unknown context (ALTER) refuses only the bare name the writer provably nulls. Two existing unit tests (OnChangeSurvivesBuilder, DynamicTextV3_AttributeBinds) built inputs with NO entity and passed — the second asserted a bare `Title` AttributeRef counted as 'bound', i.e. it pinned the bug: when a fixture has no entity context, ask what the writer does with its output. The `$P/Attr` drop was found only by dumping the control page, not from the report — print the AST value type with a probe test before assuming a spelling reaches the builder. Evidence: 22 mxbuild errors before on the probe matrix; after, every case refused with nothing written, controls (dataview/listview/gallery/datagrid/snippet dataview/ALTER into dataview) 0 errors, 17/17 stock pages + 4/4 snippets describe→exec round trip.", "refs": ["MDL-WIDGET34"], "ce": ["CE0544", "CE7005", "CE0402", "CE0642", "CE2421", "CE1365", "CE7247", "CE7006"]} {"area":"mdl/executor","date":"2026-09-25","symptom":"`alter page FeedbackModule.ShareFeedback_Logo { insert after textBox1 { image zzImg (ImageType: imageUrl, ImageUrl: '{1}', ImageUrlParams: [{1} = ImageB64]) } }` passed `check --references`; exec wrote a bare AttributeRef and `mx check` could not LOAD the project (ArgumentNullException setting 'Attribute'). Same at page top level outside any data container; a text box's `Attribute:` there is silently dropped (CE7005).","cause":"ALTER's entity context comes from the STORED document (nearest enclosing data source, or a flow source's return type via resolveDataSourceFlowEntity). With the flow missing (Feedback v4.0.2 ships no DS_FeedbackForm) or no container at all, entityContext is \"\" and resolveAttributePath returns the bare name. CREATE PAGE refused this at check time (relaxExcludedWidgetRefs/unscopedBindings, #678); ALTER's check never opened the document, so nothing could know the scope.","file":"mdl/executor/validate_alter_unscoped.go, mdl/executor/cmd_alter_page.go (alterEntityContext), mdl/executor/validate.go (bindingsWithoutScope)","insight":"For ALTER, scope is a property of the stored document, not the statement: a check-time question about it must open the document (OpenPageForMutation, never Save; validate_alter_set.go already does this) and ask through the SAME function exec uses, so the INSERT/REPLACE entity resolution was lifted into alterEntityContext rather than restated, and the binding walk lifted out of unscopedBindings (bindingsWithoutScope) rather than copied. Controls that keep it from blocking working scripts: skip a target the stored doc lacks (added earlier in the script), a flow the script declares with an entity return (sc.flowParams), DataGrid2 column and list-view-template paths, and documents the script creates. Reproduce with a Studio Pro-authored page whose flow is genuinely absent.","refs":["#678","#685"]} {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`describe page` on a File Uploader (files mode) emits `DataSource: association …`, and exec of that output fails: \"widget `upFiles` (fileuploader) exposes 2 datasources, so a generic `datasource:` clause is ambiguous — name the one you mean: associatedFiles, associatedImages\"", "cause": "DESCRIBE chose generic vs named by counting CONFIGURED datasources (namedCustomWidgetDataSources drops unset ones, so files mode = 1), while the builder's refuseAmbiguousGenericDataSource counts DECLARED datasource mappings (a generated def maps every top-level datasource = 2). Two sides of one round trip deciding the same question from different evidence.", "file": "mdl/executor/cmd_pages_describe_parse.go", "insight": "When describe and build each decide 'is this ambiguous?', they must count the same set. Fix read the DECLARED count from the stored schema (PropertyTypes with ValueType.Type=DataSource, excluding IsLinked) and excluded widgets with an embedded .def.json — those are hand-written and pick one datasource mapping per mode, so a database-mode ComboBox (two declared) must keep the generic clause. The #956 bug-test script itself authored the refused generic clause on a File Uploader: a bug-test that only runs `mxcli check` without a project can't see an exec-time refusal, so grep bug-tests for the old spelling whenever a builder starts refusing one.", "refs": ["mendixlabs/mxcli#1199", "mendixlabs/mxcli#956", "mendixlabs/mxcli#1109"], "rules": []} +{"area": "mdl/executor", "date": "2026-09-26", "symptom": "`create published rest service` with `GET 'status' microflow M.GetStatus` where the microflow takes `$orderNumber: String` passed `check`, exec said created, and mx check failed with CE0350 (parameter not an operation parameter); a `{id}` path bound to `$id: Integer` failed CE6539; an object or FileDocument parameter failed CE0350. A generated app parsed `HttpRequest/Uri` by hand to get a query parameter.", "cause": "publishedRestOperationToGen built Rest$RestOperationParameter only from the path's {name} placeholders, always as DataTypes$StringType, and never read the operation's microflow. Studio Pro derives every operation parameter from the microflow: path name -> Path, object/list -> Body, System.HttpRequest/HttpResponse -> none, anything else -> Query.", "file": "mdl/executor/cmd_published_rest.go", "insight": "When MDL has no syntax for part of a document, check whether Studio Pro derives that part from something MDL does write (here: the microflow's parameters). Deriving it at write time needs no new syntax and keeps describe -> exec a fixed point. mx check on a one-operation repro names the gap in seconds; `mxcli check` cannot see it without the microflow.", "refs": ["mendixlabs/mxcli#1206"]} diff --git a/CHANGELOG.md b/CHANGELOG.md index 520d9cc6b2..039580aee8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **A published REST operation now gets the parameters Studio Pro gives it** (mendixlabs/mxcli#1206) — `create published rest service` wrote only the path's `{name}` placeholders, each as a String, so any other microflow parameter failed `mx check` with CE0350 and an `{id}` bound to an Integer failed CE6539. MDL has no syntax for operation parameters, so a query parameter or an upload could not be built at all; a generated app parsed `HttpRequest/Uri` by hand instead. `create` and `alter` now derive them from the microflow as Studio Pro does: a parameter named in the path is a path parameter with the microflow parameter's type, an object or a list is the body, `System.HttpRequest` / `HttpResponse` are left out, and anything else is a query parameter. Measured on Mendix 11.12.1: the repro in `mdl-examples/bug-tests/published-rest-operation-parameters.mdl` went from 4 errors to 0, `GET …/status?orderNumber=ORD%2F2026%2F0012&count=3` answers `ORD/2026/0012:3` at runtime, and re-executing `describe` output leaves the service unchanged. A service written before its microflow still gets only its path parameters, and `exec` now says so. - **Keyword operators were fused in some stored expressions** — a page action's microflow argument `Flag: $a and $b` was stored as `$aand$b`, `if $x then 'a' else 'b'` as `if$xthen'a'else'b'`; the same in `contentparams` values, `send rest request … with (…)` parameters and a dynamic `execute database query`. Those four places took the expression's text without its whitespace; literals, `+` and `$currentObject` were unaffected, which is why it went unnoticed, and `check --references` passed. Measured by decoding the stored units on a Mendix 11.14.0 project. The expression is now stored as written, with MDL comments removed, as the microflow expression sites already did. - **An input bound to an attribute outside any data container was written with no binding** — `textbox t (Attribute: FullName)` at the top of a page (or of a snippet, or inside a plain container) has no entity to qualify the name with, and the writer stored it as `AttributeRef: null`. Plain `check` passed, `exec --no-check` and `alter page … insert` at page level said success, and mxbuild 11.13.0 failed the page with CE0544 "This widget can only function inside a data context" + CE7005 (text box, text area, date picker, check box, radio buttons, drop-down), CE0402 (dynamic text `Attribute:`) or CE0642 (combo box). A qualified attribute there is stored and fails the same way (CE0544 / CE2421 / CE1365 / CE7247 "Move this widget into a data container"). `Attribute: $P/Name` and `Attribute: $currentObject/Name` never parsed as an attribute path and were dropped even inside a data view. `check` now reports all three as **MDL-WIDGET34** (no project needed), and the page builder refuses them with the widget named, so nothing is written; `alter page` refuses a bare name it has no entity for. Place the widget in a data view, list view, gallery or data grid and bind the attribute by name. - **An expression property written in brackets was silently dropped** (mendixlabs/mxcli#750) — `dynamicclasses: [ if $currentObject/Featured then 'a' else 'b' ]`, the spelling #750 proposes, parsed as a list that no writer reads: `check` was clean, `exec` said `Created page`, and the widget was stored with no dynamic class. `alter page … set DynamicClasses = [ … ]` said `Altered page` and changed nothing, and a column's `DynamicCellClass` stored the list's text — tokens fused, `[if$x/Ythen'a'else'b']` — as its expression. Measured on a copy of a Mendix 11.14.0 project with the pre-fix binary. `mxcli check` now reports **MDL-WIDGET32** for `DynamicClasses` and `DynamicCellClass` written as a list (no project needed), and ALTER refuses it, so `check -p` reports that too. Write the expression quoted. diff --git a/cmd/mxcli/syntax/features_integration.go b/cmd/mxcli/syntax/features_integration.go index cb98e9b715..39644c2ca6 100644 --- a/cmd/mxcli/syntax/features_integration.go +++ b/cmd/mxcli/syntax/features_integration.go @@ -310,9 +310,10 @@ func init() { Keywords: []string{ "create published rest", "publish rest", "rest resource", "rest operation", "microflow", "path parameter", + "query parameter", "body parameter", "operation parameter", "grant access", "revoke access", }, - 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 }\n};\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;", + 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 }\n};\n\n-- Operation parameters come from the microflow's parameters, as in Studio Pro:\n-- a parameter named in the path ({id}) is a path parameter, an object or a list\n-- is the body, System.HttpRequest / HttpResponse are the request and response,\n-- and any other parameter is a query parameter: $orderNumber: String behind\n-- GET 'status' answers /status?orderNumber=... Create the microflow first.\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: "CREATE 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;", SeeAlso: []string{"rest", "rest.consumed"}, }) diff --git a/docs-site/src/examples/rest-integration.md b/docs-site/src/examples/rest-integration.md index 49afe92195..1749a9ee8b 100644 --- a/docs-site/src/examples/rest-integration.md +++ b/docs-site/src/examples/rest-integration.md @@ -322,7 +322,22 @@ CREATE PUBLISHED REST SERVICE Module.OrderAPI ( }; ``` -**Operation paths:** Use empty string `''` for the root, `'{paramName}'` for path parameters. Do NOT start or end with `/`. Path parameters must match a microflow parameter name exactly (case-sensitive) — e.g., `'{id}'` requires the microflow to declare `$id: String`. +**Operation paths:** Use empty string `''` for the root, `'{paramName}'` for path parameters. Do NOT start or end with `/`. Path parameters must match a microflow parameter name exactly (case-sensitive) — e.g., `'{id}'` requires the microflow to declare `$id`, of any primitive type. + +**Operation parameters** come from the microflow's parameters, the way Studio Pro derives them: a parameter named in the path is a path parameter, an object or a list is the body, `System.HttpRequest` and `System.HttpResponse` give the microflow the request and the response, and any other parameter is a query parameter. Create the microflow before the service; a service written first gets only its path parameters, and says so. + +```sql +-- GET rest/orders/v1/orders/status?orderNumber=ORD%2F2026%2F0012 +create microflow Module.PRS_GetOrderStatus ($orderNumber: String) +returns String as $Status +begin + retrieve $Order from Module.Order where [OrderNumber = $orderNumber] limit 1; + declare $Status String = getCaption($Order/Status); + return $Status; +end; +/ +-- in the service: GET 'status' MICROFLOW Module.PRS_GetOrderStatus; +``` ### Multiple Resources diff --git a/mdl-examples/bug-tests/published-rest-operation-parameters.mdl b/mdl-examples/bug-tests/published-rest-operation-parameters.mdl new file mode 100644 index 0000000000..772285e195 --- /dev/null +++ b/mdl-examples/bug-tests/published-rest-operation-parameters.mdl @@ -0,0 +1,60 @@ +-- ============================================================================ +-- mendixlabs/mxcli#1206 — a published REST operation gets its parameters from its microflow +-- ============================================================================ +-- +-- Studio Pro derives an operation's parameters from its microflow: a parameter +-- named in the path is a path parameter, an object or a list is the body, +-- System.HttpRequest / HttpResponse are the request and the response, and any +-- other parameter is a query parameter. mxcli wrote only the path placeholders, +-- each as a String, so mx check failed this service with CE0350 (orderNumber, +-- count, verbose, file) and CE6539 (the Integer id). +-- +-- Verified on Mendix 11.12.1: 4 errors with v0.24.0, 0 with the fix, and +-- GET rest/orders/v1/orders/status?orderNumber=ORD%2F2026%2F0012&count=3 +-- answers ORD/2026/0012:3 at runtime. +-- ============================================================================ + +create module BugRestParams; + +create persistent entity BugRestParams.Upload extends System.FileDocument ( + Label: String(100) +); + +create microflow BugRestParams.GetStatus ($orderNumber: String, $count: Integer, $httpRequest: System.HttpRequest) +returns String as $Result +begin + declare $Result String = $orderNumber + ':' + toString($count); + return $Result; +end; +/ + +create microflow BugRestParams.GetById ($id: Integer, $verbose: Boolean) +returns String as $Result +begin + declare $Result String = toString($id); + return $Result; +end; +/ + +create microflow BugRestParams.PutFile ($file: BugRestParams.Upload) +returns String as $Result +begin + declare $Result String = 'ok'; + return $Result; +end; +/ + +create published rest service BugRestParams.PRS_Orders ( + Path: 'rest/orders/v1', + Version: '1.0.0', + ServiceName: 'Orders' +) +{ + resource 'orders' { + GET 'status' microflow BugRestParams.GetStatus; + GET 'items/{id}' microflow BugRestParams.GetById; + POST 'upload' microflow BugRestParams.PutFile; + } +}; +/ + diff --git a/mdl/backend/modelsdk/published_rest_params_test.go b/mdl/backend/modelsdk/published_rest_params_test.go new file mode 100644 index 0000000000..0862f81d1b --- /dev/null +++ b/mdl/backend/modelsdk/published_rest_params_test.go @@ -0,0 +1,116 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "testing" + + "go.mongodb.org/mongo-driver/bson" + + "github.com/mendixlabs/mxcli/model" +) + +// TestCreatePublishedRestService_WritesDerivedParameters: the parameters the executor +// derived from the microflow are stored as Rest$RestOperationParameter elements, each with +// its kind, its microflow parameter and that parameter's type. An operation whose +// microflow could not be read keeps its path placeholders as String path parameters. +func TestCreatePublishedRestService_WritesDerivedParameters(t *testing.T) { + proj := copyFixture(t) + b := New() + if err := b.Connect(proj); err != nil { + t.Fatalf("connect: %v", err) + } + mod, err := b.GetModuleByName("MyFirstModule") + if err != nil || mod == nil { + t.Fatalf("GetModuleByName: %v", err) + } + svc := &model.PublishedRestService{ + ContainerID: mod.ID, + Name: "ZzParams", + Path: "rest/zz/v1", + Version: "1.0.0", + Resources: []*model.PublishedRestResource{{ + Name: "orders", + Operations: []*model.PublishedRestOperation{ + {HTTPMethod: "GET", Path: "items/{id}", Microflow: "MyFirstModule.GetItem", + OperationParameters: []*model.PublishedRestOperationParameter{ + {Name: "id", ParameterType: "Path", DataType: "Integer"}, + {Name: "orderNumber", ParameterType: "Query", DataType: "String"}, + {Name: "item", ParameterType: "Body", DataType: "Object", QualifiedName: "MyFirstModule.Item"}, + }}, + {HTTPMethod: "GET", Path: "legacy/{code}", Microflow: "MyFirstModule.Unknown"}, + }, + }}, + } + if err := b.CreatePublishedRestService(svc); err != nil { + t.Fatalf("CreatePublishedRestService: %v", err) + } + if err := b.Disconnect(); err != nil { + t.Fatalf("disconnect: %v", err) + } + + var doc bson.M + if err := bson.Unmarshal(readUnitBytes(t, proj, string(svc.ID)), &doc); err != nil { + t.Fatalf("unmarshal unit: %v", err) + } + ops := paramItems(t, paramItems(t, doc, "Resources")[0], "Operations") + + derived := paramItems(t, ops[0], "Parameters") + want := []struct{ name, kind, typ, ref string }{ + {"id", "Path", "DataTypes$IntegerType", ""}, + {"orderNumber", "Query", "DataTypes$StringType", ""}, + {"item", "Body", "DataTypes$ObjectType", "MyFirstModule.Item"}, + } + if len(derived) != len(want) { + t.Fatalf("parameters = %d, want %d", len(derived), len(want)) + } + for i, w := range want { + p := derived[i] + typ := paramDoc(t, p["Type"]) + if p["Name"] != w.name || p["ParameterType"] != w.kind || typ["$Type"] != w.typ || + p["MicroflowParameter"] != "MyFirstModule.GetItem."+w.name { + t.Errorf("parameter %d = %v (Type %v), want %+v", i, p, typ, w) + } + if w.ref != "" && typ["Entity"] != w.ref { + t.Errorf("parameter %s entity = %v, want %s", w.name, typ["Entity"], w.ref) + } + } + + fallback := paramItems(t, ops[1], "Parameters") + if len(fallback) != 1 || fallback[0]["Name"] != "code" || fallback[0]["ParameterType"] != "Path" || + paramDoc(t, fallback[0]["Type"])["$Type"] != "DataTypes$StringType" { + t.Errorf("fallback parameters = %v, want one String path parameter 'code'", fallback) + } +} + +// paramItems returns the elements of a marker-prefixed BSON list property. +func paramItems(t *testing.T, doc map[string]any, key string) []map[string]any { + t.Helper() + arr, ok := doc[key].(bson.A) + if !ok || len(arr) < 1 { + t.Fatalf("%s = %#v, want a marker-prefixed list", key, doc[key]) + } + var out []map[string]any + for _, item := range arr[1:] { + out = append(out, paramDoc(t, item)) + } + return out +} + +func paramDoc(t *testing.T, v any) map[string]any { + t.Helper() + switch m := v.(type) { + case bson.M: + return m + case bson.D: + out := make(map[string]any, len(m)) + for _, e := range m { + out[e.Key] = e.Value + } + return out + case map[string]any: + return m + } + t.Fatalf("value has type %T, want a document", v) + return nil +} diff --git a/mdl/backend/modelsdk/published_rest_write.go b/mdl/backend/modelsdk/published_rest_write.go index f314e66121..4739a62e07 100644 --- a/mdl/backend/modelsdk/published_rest_write.go +++ b/mdl/backend/modelsdk/published_rest_write.go @@ -13,6 +13,7 @@ import ( "github.com/mendixlabs/mxcli/modelsdk/element" mmpr "github.com/mendixlabs/mxcli/modelsdk/mpr" "github.com/mendixlabs/mxcli/modelsdk/property" + "github.com/mendixlabs/mxcli/sdk/microflows" ) func init() { @@ -164,18 +165,25 @@ func publishedRestOperationToGen(op *model.PublishedRestOperation) element.Eleme addStr(g, "ExportMapping", "") addStr(g, "ImportMapping", "") addStr(g, "ObjectHandlingBackup", "Create") - // Path parameters are auto-extracted from {name} placeholders and wired to the - // matching microflow parameter (Module.Microflow.name) — without that wiring - // mx check raises CE6538 / CE0350. - params := make([]element.Element, 0) - for _, name := range extractPathParams(op.Path) { + // The parameters the executor derived from the microflow (path, query, body). When it + // could not read the microflow, only the {name} placeholders of the path are known; + // those are written as String path parameters wired to the microflow parameter of + // that name, since without that wiring mx check raises CE6538 / CE0350. + opParams := op.OperationParameters + if len(opParams) == 0 { + for _, name := range op.PathParameterNames() { + opParams = append(opParams, &model.PublishedRestOperationParameter{Name: name, ParameterType: "Path", DataType: "String"}) + } + } + params := make([]element.Element, 0, len(opParams)) + for _, param := range opParams { p := newElem("Rest$RestOperationParameter", "") - addStr(p, "Name", name) - addPart(p, "Type", newElem("DataTypes$StringType", "")) - addStr(p, "ParameterType", "Path") + addStr(p, "Name", param.Name) + addPart(p, "Type", microflowDataTypeToGen(operationParameterDataType(param))) + addStr(p, "ParameterType", param.ParameterType) mfParam := "" if op.Microflow != "" { - mfParam = op.Microflow + "." + name + mfParam = op.Microflow + "." + param.Name } addStr(p, "MicroflowParameter", mfParam) addStr(p, "Description", "") @@ -197,22 +205,32 @@ func addByNameRefList(b *element.Base, name, targetType string, qnames []string) } } -// extractPathParams returns parameter names from {param} placeholders in a path. -func extractPathParams(path string) []string { - var names []string - for { - start := strings.Index(path, "{") - if start < 0 { - break - } - end := strings.Index(path[start:], "}") - if end < 0 { - break - } - names = append(names, path[start+1:start+end]) - path = path[start+end+1:] +// operationParameterDataType is the microflow data type an operation parameter carries. +func operationParameterDataType(p *model.PublishedRestOperationParameter) microflows.DataType { + switch p.DataType { + case "Boolean": + return µflows.BooleanType{} + case "Integer": + return µflows.IntegerType{} + case "Long": + return µflows.LongType{} + case "Decimal": + return µflows.DecimalType{} + case "DateTime": + return µflows.DateTimeType{} + case "Date": + return µflows.DateType{} + case "Binary": + return µflows.BinaryType{} + case "Enumeration": + return µflows.EnumerationType{EnumerationQualifiedName: p.QualifiedName} + case "Object": + return µflows.ObjectType{EntityQualifiedName: p.QualifiedName} + case "List": + return µflows.ListType{EntityQualifiedName: p.QualifiedName} + default: + return µflows.StringType{} } - return names } // httpMethodToMendix converts an HTTP method name to Mendix casing. diff --git a/mdl/executor/cmd_published_rest.go b/mdl/executor/cmd_published_rest.go index e9f6a3074b..f35b941ffc 100644 --- a/mdl/executor/cmd_published_rest.go +++ b/mdl/executor/cmd_published_rest.go @@ -11,6 +11,7 @@ import ( "github.com/mendixlabs/mxcli/mdl/ast" mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" ) // listPublishedRestServices handles SHOW PUBLISHED REST SERVICES [IN module] command. @@ -248,6 +249,7 @@ func execCreatePublishedRestService(ctx *ExecContext, s *ast.CreatePublishedRest svc.Resources = append(svc.Resources, resource) } + deriveOperationParameters(ctx, svc) if existing != nil { if s.Folder == "" { svc.ContainerID = existing.ContainerID @@ -272,6 +274,62 @@ func execCreatePublishedRestService(ctx *ExecContext, s *ast.CreatePublishedRest return nil } +// deriveOperationParameters gives every operation the parameters Studio Pro derives from +// its microflow: a parameter named in the path is a path parameter, an object or a list is +// the body, and any other parameter is a query parameter. System.HttpRequest and +// System.HttpResponse parameters are the request and response themselves, not operation +// parameters. Without the query and body parameters mx check reports CE0350 for every +// microflow parameter the path does not name. +func deriveOperationParameters(ctx *ExecContext, svc *model.PublishedRestService) { + for _, resource := range svc.Resources { + for _, op := range resource.Operations { + op.OperationParameters = nil + if op.Microflow == "" { + continue + } + mf := findMicroflowByQualifiedName(ctx, op.Microflow) + if mf == nil { + fmt.Fprintf(ctx.Output, "Warning: microflow %s not found, so operation %s %s gets only its path parameters -- "+ + "create the microflow before the service, or its other parameters fail mx check with CE0350\n", + op.Microflow, strings.ToUpper(op.HTTPMethod), op.Path) + continue + } + op.OperationParameters = operationParametersOf(mf, op.PathParameterNames()) + } + } +} + +// operationParametersOf maps a microflow's parameters to operation parameters. +func operationParametersOf(mf *microflows.Microflow, pathNames []string) []*model.PublishedRestOperationParameter { + inPath := make(map[string]bool, len(pathNames)) + for _, name := range pathNames { + inPath[name] = true + } + var params []*model.PublishedRestOperationParameter + for _, p := range mf.Parameters { + param := &model.PublishedRestOperationParameter{Name: p.Name, ParameterType: "Query"} + if p.Type != nil { + param.DataType = p.Type.GetTypeName() + } + switch t := p.Type.(type) { + case *microflows.ObjectType: + if t.EntityQualifiedName == "System.HttpRequest" || t.EntityQualifiedName == "System.HttpResponse" { + continue + } + param.ParameterType, param.QualifiedName = "Body", t.EntityQualifiedName + case *microflows.ListType: + param.ParameterType, param.QualifiedName = "Body", t.EntityQualifiedName + case *microflows.EnumerationType: + param.QualifiedName = t.EnumerationQualifiedName + } + if inPath[p.Name] { + param.ParameterType = "Path" + } + params = append(params, param) + } + return params +} + // execDropPublishedRestService deletes a published REST service. func execDropPublishedRestService(ctx *ExecContext, s *ast.DropPublishedRestServiceStmt) error { if !ctx.ConnectedForWrite() { @@ -381,6 +439,7 @@ func execAlterPublishedRestService(ctx *ExecContext, s *ast.AlterPublishedRestSe } } + deriveOperationParameters(ctx, svc) if err := ctx.Backend.UpdatePublishedRestService(svc); err != nil { return mdlerrors.NewBackend("alter published rest service", err) } diff --git a/mdl/executor/cmd_published_rest_params_test.go b/mdl/executor/cmd_published_rest_params_test.go new file mode 100644 index 0000000000..a9b4b5a8bb --- /dev/null +++ b/mdl/executor/cmd_published_rest_params_test.go @@ -0,0 +1,169 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// publishedRestService returns a service with one operation, bound to microflow Api.Op. +func publishedRestService(path string) *model.PublishedRestService { + return &model.PublishedRestService{ + Name: "Orders", + Resources: []*model.PublishedRestResource{{ + Name: "orders", + Operations: []*model.PublishedRestOperation{{HTTPMethod: "GET", Path: path, Microflow: "Api.Op"}}, + }}, + } +} + +// TestDeriveOperationParameters_AsStudioPro: a microflow parameter named in the path is a +// path parameter, an object or a list is the body, System.HttpRequest / HttpResponse are +// not operation parameters, and anything else is a query parameter. Before, only the path +// placeholders were written, and mx check reported CE0350 for every other parameter. +func TestDeriveOperationParameters_AsStudioPro(t *testing.T) { + mod := mkModule("Api") + mf := µflows.Microflow{ + ContainerID: mod.ID, + Name: "Op", + Parameters: []*microflows.MicroflowParameter{ + {Name: "id", Type: µflows.IntegerType{}}, + {Name: "orderNumber", Type: µflows.StringType{}}, + {Name: "status", Type: µflows.EnumerationType{EnumerationQualifiedName: "Api.Status"}}, + {Name: "item", Type: µflows.ObjectType{EntityQualifiedName: "Api.Item"}}, + {Name: "lines", Type: µflows.ListType{EntityQualifiedName: "Api.Line"}}, + {Name: "httpRequest", Type: µflows.ObjectType{EntityQualifiedName: "System.HttpRequest"}}, + {Name: "httpResponse", Type: µflows.ObjectType{EntityQualifiedName: "System.HttpResponse"}}, + }, + } + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { return []*microflows.Microflow{mf}, nil }, + } + ctx, buf := newMockCtx(t, withBackend(mb), withHierarchy(mkHierarchy(mod))) + + svc := publishedRestService("items/{id}") + deriveOperationParameters(ctx, svc) + + got := map[string]model.PublishedRestOperationParameter{} + for _, p := range svc.Resources[0].Operations[0].OperationParameters { + got[p.Name] = *p + } + want := map[string]model.PublishedRestOperationParameter{ + "id": {Name: "id", ParameterType: "Path", DataType: "Integer"}, + "orderNumber": {Name: "orderNumber", ParameterType: "Query", DataType: "String"}, + "status": {Name: "status", ParameterType: "Query", DataType: "Enumeration", QualifiedName: "Api.Status"}, + "item": {Name: "item", ParameterType: "Body", DataType: "Object", QualifiedName: "Api.Item"}, + "lines": {Name: "lines", ParameterType: "Body", DataType: "List", QualifiedName: "Api.Line"}, + } + if len(got) != len(want) { + t.Fatalf("parameters = %v, want %v", got, want) + } + for name, w := range want { + if got[name] != w { + t.Errorf("parameter %s = %+v, want %+v", name, got[name], w) + } + } + if buf.Len() != 0 { + t.Errorf("unexpected output: %q", buf.String()) + } +} + +// TestDeriveOperationParameters_WarnsWhenTheMicroflowIsMissing: a service written before +// its microflow cannot know the microflow's parameters. It keeps the path parameters, as +// before, and says why the rest will fail mx check. +func TestDeriveOperationParameters_WarnsWhenTheMicroflowIsMissing(t *testing.T) { + mod := mkModule("Api") + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { return nil, nil }, + } + ctx, buf := newMockCtx(t, withBackend(mb), withHierarchy(mkHierarchy(mod))) + + svc := publishedRestService("items/{id}") + deriveOperationParameters(ctx, svc) + + if n := len(svc.Resources[0].Operations[0].OperationParameters); n != 0 { + t.Errorf("derived %d parameters from a missing microflow", n) + } + assertContainsStr(t, buf.String(), "microflow Api.Op not found") + assertContainsStr(t, buf.String(), "CE0350") +} + +// TestCreateAndAlterPublishedRestService_DeriveParameters: both statements that write a +// service hand the backend operations that carry their derived parameters. ALTER writes +// every operation again, so it has to derive them for the operations it did not touch too. +func TestCreateAndAlterPublishedRestService_DeriveParameters(t *testing.T) { + mod := mkModule("Api") + mf := µflows.Microflow{ + ContainerID: mod.ID, + Name: "Op", + Parameters: []*microflows.MicroflowParameter{{Name: "orderNumber", Type: µflows.StringType{}}}, + } + var written *model.PublishedRestService + var stored []*model.PublishedRestService + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { return []*microflows.Microflow{mf}, nil }, + ListPublishedRestServicesFunc: func() ([]*model.PublishedRestService, error) { + return stored, nil + }, + CreatePublishedRestServiceFunc: func(svc *model.PublishedRestService) error { + written = svc + return nil + }, + UpdatePublishedRestServiceFunc: func(svc *model.PublishedRestService) error { + written = svc + return nil + }, + } + h := mkHierarchy(mod) + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h)) + op := func() []*ast.PublishedRestOperationDef { + return []*ast.PublishedRestOperationDef{{HTTPMethod: "GET", Path: "status", + Microflow: ast.QualifiedName{Module: "Api", Name: "Op"}}} + } + queryParams := func(svc *model.PublishedRestService) []string { + var names []string + for _, res := range svc.Resources { + for _, o := range res.Operations { + for _, p := range o.OperationParameters { + if p.ParameterType == "Query" { + names = append(names, res.Name+":"+p.Name) + } + } + } + } + return names + } + + assertNoError(t, execCreatePublishedRestService(ctx, &ast.CreatePublishedRestServiceStmt{ + Name: ast.QualifiedName{Module: "Api", Name: "Orders"}, Path: "rest/orders/v1", + Resources: []*ast.PublishedRestResourceDef{{Name: "orders", Operations: op()}}, + })) + if got := queryParams(written); len(got) != 1 || got[0] != "orders:orderNumber" { + t.Fatalf("create wrote query parameters %v, want [orders:orderNumber]", got) + } + + // The service as it reads back: the reader does not read operation parameters. + written.ID = "prs-1" + written.ContainerID = mod.ID + for _, o := range written.Resources[0].Operations { + o.OperationParameters = nil + } + stored = []*model.PublishedRestService{written} + assertNoError(t, execAlterPublishedRestService(ctx, &ast.AlterPublishedRestServiceStmt{ + Name: ast.QualifiedName{Module: "Api", Name: "Orders"}, + Actions: []ast.PublishedRestAlterAction{&ast.PublishedRestAddResourceAction{Resource: &ast.PublishedRestResourceDef{Name: "more", Operations: op()}}}, + })) + got := queryParams(written) + if len(got) != 2 || got[0] != "orders:orderNumber" || got[1] != "more:orderNumber" { + t.Fatalf("alter wrote query parameters %v, want [orders:orderNumber more:orderNumber]", got) + } +} diff --git a/model/types.go b/model/types.go index 6e7d30868f..54719a3bff 100644 --- a/model/types.go +++ b/model/types.go @@ -5,6 +5,7 @@ package model import ( "encoding/json" + "strings" "time" "go.mongodb.org/mongo-driver/bson" @@ -808,6 +809,39 @@ type PublishedRestOperation struct { Microflow string `json:"microflow,omitempty"` Deprecated bool `json:"deprecated,omitempty"` Parameters []string `json:"parameters,omitempty"` // path parameter names extracted from {param} in Path + // OperationParameters are the operation's parameters as Studio Pro derives them from + // its microflow. Empty when the microflow could not be read; the writer then writes + // the path placeholders alone, as String path parameters. + OperationParameters []*PublishedRestOperationParameter `json:"operationParameters,omitempty"` +} + +// PathParameterNames returns the names of the {name} placeholders in the operation's +// path, in order. +func (op *PublishedRestOperation) PathParameterNames() []string { + var names []string + path := op.Path + for { + start := strings.Index(path, "{") + if start < 0 { + break + } + end := strings.Index(path[start:], "}") + if end < 0 { + break + } + names = append(names, path[start+1:start+end]) + path = path[start+end+1:] + } + return names +} + +// PublishedRestOperationParameter is one parameter of a published REST operation. It +// binds to the operation microflow's parameter of the same name. +type PublishedRestOperationParameter struct { + Name string `json:"name"` + ParameterType string `json:"parameterType"` // "Path", "Query" or "Body" + DataType string `json:"dataType"` // the microflow parameter's type: "String", "Integer", "Object", ... + QualifiedName string `json:"qualifiedName,omitempty"` // the entity of an Object or List, or the enumeration } // ============================================================================