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 } // ============================================================================