diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 5a984b569..bcb0890e9 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -405,3 +405,11 @@ extracting `OffsetExpression`/`LimitExpression`. | A suite passes under `mxcli test --attach` and fails under `--local`, with assertions that depend on startup state (a loaded cache, seeded reference data) seeing zero rows | The `--local` runner pointed after-startup at its own registration microflow and did **not** chain the project's own, so the app's startup logic never ran. It was a deliberate choice (a known baseline) but was invisible: the run printed only `After-startup set to MxTest.RegisterEndpoint`, never that the user's microflow had been displaced | `cmd/mxcli/testrunner/runner.go` (`runEndpoint`), `cmd/mxcli/testrunner/cleanup_strategy.go` (`describeStartup`) | Capture project state **before** generating the endpoint MDL, and pass `state.afterStartup` to `GenerateEndpointMDL` so the generated flow chains it — the hosted `--test-endpoint` path already did this, and the mismatch between the two was the bug. Add `--skip-app-startup` for a deterministic empty baseline, and always print which of the two happened. Note the startup microflow's writes run at boot, outside any test transaction, so `@cleanup rollback` does not undo them. mxcli-formula1 findings #19 | | `mxcli test tests/ -p app/App.mpr --list` fails with `stat tests/: no such file or directory` while the same command without `--list` runs fine | The `--list` branch passed raw `args` to `ListTests`, bypassing `resolveTestPaths` — so a path relative to the project (rather than the working directory) resolved for execution but not for listing | `cmd/mxcli/cmd_test_run.go` (the `if list` branch) | Pass `resolveTestPaths(args, projectPath)` there too. When a command has two entry points into the same input, check both go through the same path resolution. mxcli-formula1 findings #15 | | After `SET` became optional, `mx check` on a project built from `02-microflow-examples.mdl` reports six errors that no MDL change caused: `[CE0109] "Undefined variable 'ProductList.Price'."` at four Aggregate list activities, and `[CE0015] "Aggregate function must specify a valid attribute."` at the expression-based one. Identical on both engines. `mxcli check` on the same script is silent | Two conversions existed for one syntax. `$Sum = sum($List.Price)` used to reach the dedicated `aggregateListStatement` rule; making `SET` optional put `setStatement` — alternative 5 of ~50 — in front of it, so ANTLR matched the lower-numbered alternative and the statement fell through to `buildSetStatement`'s fallback conversion, which joined list and attribute into one name and dropped the per-item expression entirely. Underneath, `buildListAggregateAsFunction` never appended the expression argument, so the SET path could not have seen it either | `mdl/grammar/domains/MDLMicroflow.g4` (`setStatement` moved LAST in `microflowStatement`), `mdl/visitor/visitor_microflow_statements.go` (`buildSetAggregate` replaces `extractVariableAndAttribute`), `mdl/visitor/visitor_microflow_expression.go` (`buildListAggregateAsFunction` appends the expression argument) | **A permissive alternative belongs last.** `$X = ` overlaps every `VARIABLE EQUALS ` statement in the rule — aggregates, list operations, RANGE — and ANTLR's ALL(*) picks the lowest-numbered alternative that matches, so a new general form silently steals from every specific one above it. **The measurement trap that let this ship**: the `SET?` change was swept with a control binary over every `mdl-examples/**/*.mdl` and found no difference — but with `mxcli check`, which parses and validates and never serializes. This defect lives between the AST and the BSON, where only `exec` + `mx check` can see it. Sweeping with `check` proves the grammar still *parses*; it proves nothing about what the visitor *builds*. For a grammar change, the control sweep must run the integration gate (`go test -tags integration -run TestMxCheck_DoctypeScripts`), not `check`. Fix proven by reverting both halves and watching the new tests fail with the reported symptom. Tests `mdl/visitor/visitor_microflow_aggregate_test.go`. Upstream CI on the ako→mendixlabs sync PR | +| A published service will not build: every whole-number attribute is `[CE5016] "Attribute … has type Integer, but is published as Edm.Int32"`, and an exposed enumeration adds CE5016 plus `[CE4583] "Enumeration 'X' is not published in this service."` | `mendixAttrTypeToEdm` mapped Integer→Int32 (Mendix publishes it as **Int64**, same as Long), and the enum path wrote `Edm.String` while `EnumerationAsString` was hardcoded `false` — the one combination Mendix rejects, since with the flag false it wants the enumeration published as its own EDM enum type. The function's own comment flagged the unverified rows, and the existing unit test *pinned the wrong answer* | `mdl/executor/cmd_odata.go` (`mendixAttrTypeToEdm`, `enumPublishedAsString`, `publishedAttrType`), `model/types.go` (`PublishedMember.EnumerationAsString`), `mdl/backend/modelsdk/odata_write.go` + `sdk/mpr/writer_odata.go` (stop hardcoding the flag) | **Let mxbuild adjudicate the whole table at once**: publish one attribute of every Mendix type in one service and read the CE5016s off the build. That found Integer (reported) *and* Enumeration (only suspected), and confirmed String/Long/Decimal/Boolean/DateTime were already right — five verified rows for one build. Binary turns out to be unpublishable at all (CE5013), whatever type you give it. **A type and a flag that only work as a pair must travel as a pair** — `Edm.String` is ambiguous between String and a flattened enum, so the flag is the only thing distinguishing them and it belongs on the same struct. Watch for an existing test that encodes the bug: this one asserted `Edm.Int32`, so the fix *failed the suite* until the assertion was corrected. Tests `cmd_contract_test.go`, `cmd_odata_edm_type_test.go`. mxcli-formula1 #16 | +| `create or modify external entity Mod.E (… Countable: false)` — touching only an entity-level property — detonates every attribute: `[CE6612] "Attribute 'circuitId' of external entity 'Stg_Circuit' is not supported."`, one per attribute, leaving a project that cannot build | Not the executor: it already preserves attributes it was not asked to change (`if len(attrs) > 0`). One layer down, `attributeFromGen` handled `StoredValue` and `OqlViewValue` but **not** `Rest$ODataMappedValue`, so every attribute of an external entity read back with no `RemoteName`, and the writer's `isExternal && a.RemoteName != ""` arm then fell through to a plain StoredValue on the next read-modify-write | `mdl/backend/modelsdk/domainmodel.go` (`attributeFromGen` gains the `ODataMappedValue` / `ODataMappedPrimitiveCollectionValue` arms) | **The attribute-level half of #782**, which fixed the entity level and stopped there — when a read-modify-write loses data, check every *nesting level* of the read, not just the one named in the report. A polymorphic `Value` switch that silently ignores a variant is the shape to look for: it compiles, it reads, and it drops. Reproduce with a **local metadata file** (`MetadataUrl: './contract.xml'`) — no server needed, and the import is the same code path. Also learned here: the reported attribute *rename* (`name` → `Stg_Circuitname`) is a different thing entirely — it happens at import, from `reservedEntityAttrNames`, and `name` is **not** actually reserved (verified: Mendix builds an external entity with an attribute literally named `name`). Tests `external_entity_read_test.go`. mxcli-formula1 #25 | +| `execute database query … dynamic $Sql` reaches the runtime as the literal string `'$Sql'` — `Parser Error: syntax error at or near "$"` from the database, not from Mendix. Runtime-built SQL, and therefore query pushdown, is impossible | The builder quoted any dynamic query not already starting with a quote — right for `dynamic 'SELECT …'`, wrong for an expression — and the AST kept one `DynamicQuery` string whichever branch of the grammar produced it, so nothing downstream could tell them apart | `mdl/ast/ast_microflow.go` (`DynamicQueryIsExpression`), `mdl/visitor/visitor_microflow_actions.go` (set it in the `expr` branch), `mdl/executor/cmd_microflows_builder_calls.go` (`dynamicQueryExpression`) | **When a grammar has two alternatives that mean different things, the AST must record which one fired** — a shared field plus a "does it look quoted?" heuristic is a guess, and the workaround users find (`dynamic '' + $Sql`, which starts with a quote so the heuristic leaves it alone) is proof the heuristic is the bug. Verified by reading the stored BSON rather than by describe: `DynamicQuery\x00\x05\x00\x00\x00$Sql` — five bytes, no quotes. Tests `cmd_microflows_dynamic_query_test.go`. mxcli-formula1 #21 | +| `create odata client` against a service behind `authentication basic` prints `Warning: could not fetch $metadata: … HTTP 401`, creates the client anyway, and the following `create external entities from …` imports nothing — from a script that reports success | The statement's `HttpUsername`/`HttpPassword`/`HEADERS` are stored for the runtime, but the design-time fetch was a bare `client.Get`. The fetch failure is only a warning, so the empty client propagates silently | `mdl/executor/cmd_odata.go` (`metadataFetchAuth`, `metadataAuthFromStmt`, `fetchODataMetadata`), `mdl/ast/ast_odata.go` + `mdl/visitor/visitor_odata.go` (`HttpUsernameIsLiteral` / `HeaderDef.ValueIsLiteral`) | **Only a literal is usable at design time.** The visitor strips a quoted literal's quotes, so `'f1api'` and `Module.ApiUser` both arrive as bare strings — the AST has to record which was written, or mxcli sends a *constant's name* as the password. Unresolved names are reported instead, which is also the honest answer: mxcli has no runtime to resolve a constant against. **A warning on a step something else silently depends on needs to say what breaks next** — the message now names the empty client and the import that will do nothing. Verified against a real basic-auth server that 401s without credentials and 403s without the custom header, so both had to arrive. Tests `cmd_odata_metadata_auth_test.go`. mxcli-formula1 #23 | +| Re-running `create or modify odata service` after editing a `publish entity` block changes nothing — the served `$metadata` is identical, and only `drop odata service` + create picks the edit up | The modify branch updated the service's scalar properties and never touched `EntityTypes` / `EntitySets` | `mdl/executor/cmd_odata.go` (modify branch rebuilds published entities via `astEntityDefToModel`, and carries `AllowedModuleRoles` through) | **Replace, don't merge**: a member removed from the script has to leave the service, which merging cannot express — the script is the description of the service. **Carry through what the statement cannot express**: role grants come from a separate `grant access on odata service` and would otherwise be dropped by a modify (reported; *not* reproduced on 11.12.1 — kept as a guard, and the commit says so rather than claiming a fix). Verified: same script yields `Label as 'label'` before and `Label as 'label' (Filterable, Sortable)` after, build stays at 0 errors. Tests `cmd_odata_modify_members_test.go`. mxcli-formula1 #26 | +| `create external entities from` a contract that restricts capabilities produces a project that will not build: `'Seasons' is marked Countable=False in the OData service, but True in the app`, `'latitude' is marked Filterable=False …` — one per restricted resource or property | Insert/Update/Delete restrictions were parsed; **Count/Filter/Sort were not**, so the import had nothing to honour and defaulted all three to true — on the one command whose entire job is fidelity to the contract | `mdl/types/edmx.go` (`EdmEntitySet.Countable`, `NonFilterableProperties`, `NonSortableProperties` + the three `applyCapabilityAnnotations` arms), `mdl/executor/cmd_contract.go` | An unannotated set still means countable/filterable/sortable — **silence in a contract is not a restriction**, it is OData's own default, so `nil` and `false` must stay distinguishable (`*bool`, as with the publish-side query options). The generated entity is compared against the contract at *build* time, so anything the contract can say is something the importer must be able to read. Tests `mdl/types/edmx_test.go`. mxcli-formula1 #24 | +| A contract property called `name` is generated as `Stg_Drivername` / `Circuitname` — prefixed with the remote type. A page written against the published `$metadata` then fails with `The selected attribute 'F1Live.Drivers.name' no longer exists`, and the *same* field carries a different name in every module because the remote type names differ | `attrNameForOData` disambiguates any name in `reservedEntityAttrNames`, and `name` was on that list with the comment "Mendix system-managed attribute for the object name". It is not: Mendix builds an external entity with an attribute literally named `name` | `mdl/executor/cmd_contract.go` (`reservedEntityAttrNames` loses one entry; the import now reports the renames it does make) | **Test the whole list at once, not the reported entry.** One contract with a property per listed name, prefixing disabled, then `mx check`: CE7247 "The name 'x' is a reserved word" for `id`/`owner`/`changedBy`/`changedDate`/`createdDate`/`type`/`context`, and silence for `name`. That turns "is the list wrong?" into "which rows are wrong?" for the cost of a single build, and it *earns* the seven entries that stay rather than leaving them as folklore. Two existing tests pinned the old behaviour and had to be corrected — a hand-maintained list of platform rules will accrete guesses unless each row can point at an error code. **Migration**: a re-import renames the attribute back, so references to the prefixed name must follow. Tests `cmd_contract_reserved_test.go`. mxcli-formula1 #28 | +| `MOVE JAVA ACTION …` / `MOVE ODATA SERVICE …` is a parse error (`no viable alternative at input 'MOVEJAVA'`), and neither `CREATE` form takes a folder clause — so those documents can never leave the module root from MDL | The `moveStatement` rule listed seven doctypes and nothing else; the missing ones were never unimplemented, just unlisted | `mdl/grammar/MDLParser.g4` (two alternatives), `mdl/ast/ast.go`, `mdl/visitor/visitor_entity.go` (dispatch **and** the MOVE FOLDER discriminator), `mdl/executor/cmd_move.go`, backend `MoveJavaAction` / `MovePublishedODataService`, `sdk/mpr` exports `MoveUnitByID` | Both reduce to the existing reparent primitive — a top-level document move is one containment row, so a new doctype is a list entry plus a lookup, not new machinery. **Watch the discriminator**: `MOVE FOLDER` is told apart from a document move by the *absence* of a doctype keyword, so every keyword added to the rule must also be added to that condition or a folder move starts parsing as a document move. **Verify placement by differential count, not by reading the model**: run the script with and without the MOVE lines and diff `select ContainmentName, count(*) from Unit` — three new Folders rows (a nested path creates two) and an unchanged Documents count says reparented rather than copied or dropped. Grepping blobs for names is a trap; stock modules are full of the same words. Tests `visitor_move_doctypes_test.go`, example in `18-folder-examples.mdl` — which must sit **before** that script's `drop module`, a mistake the integration gate caught and `mxcli check` did not. mxcli-formula1 #32 | diff --git a/.claude/skills/mendix/manage-security.md b/.claude/skills/mendix/manage-security.md index 7236ffbc3..1fc8c9eae 100644 --- a/.claude/skills/mendix/manage-security.md +++ b/.claude/skills/mendix/manage-security.md @@ -95,6 +95,10 @@ create module role MyModule.Admin description 'Full administrative access'; create module role MyModule.User; create module role MyModule.Viewer description 'Read-only access'; +-- `or modify` updates an existing role's description instead of failing, so the +-- whole security script stays re-runnable rather than needing a run-once file. +create or modify module role MyModule.ApiUser description 'API consumer'; + -- Remove a module role drop module role MyModule.Viewer; ``` diff --git a/.claude/skills/mendix/organize-project.md b/.claude/skills/mendix/organize-project.md index a1a3008cf..4f28da0fc 100644 --- a/.claude/skills/mendix/organize-project.md +++ b/.claude/skills/mendix/organize-project.md @@ -168,8 +168,17 @@ move page OldModule.CustomerPage to NewModule; | Nanoflow | `folder 'path'` (keyword) | `move nanoflow ...` | | Snippet | `folder: 'path'` (property) | `move snippet ...` | | Enumeration | N/A | `move enumeration ...` | +| Constant | N/A | `move constant ...` | +| Database connection | N/A | `move database connection ...` | +| Java action | N/A | `move java action ...` | +| OData service (published) | N/A | `move odata service ...` | | Entity | N/A | `move entity ...` (module only, no folders) | +**Java actions and published OData services have no folder clause on `create`**, so +`move` is the only way to place them — before this they were stuck at the module +root forever. Both are plain document units, so the move is model-level only: it +changes containment and nothing else. + **Note:** Pages and snippets use property syntax (`folder: 'path'` inside parentheses). Microflows and nanoflows use keyword syntax (`folder 'path'` before `begin`). Entities are embedded in domain models and can only be moved to a different module (no folder support). ## Example: Reorganize a Module diff --git a/cmd/mxcli/syntax/features_misc.go b/cmd/mxcli/syntax/features_misc.go index 5bbab9c68..8d8837aa1 100644 --- a/cmd/mxcli/syntax/features_misc.go +++ b/cmd/mxcli/syntax/features_misc.go @@ -211,6 +211,8 @@ SHOW STRUCTURE DEPTH 1 ALL;`, "move folder", "drop folder", }, Syntax: `MOVE Module.Name TO FOLDER 'Path'; +-- doctype: PAGE | MICROFLOW | NANOFLOW | SNIPPET | ENUMERATION | CONSTANT +-- | DATABASE CONNECTION | JAVA ACTION | ODATA SERVICE | ENTITY | FOLDER MOVE Module.Name TO TargetModule; MOVE OldModule.Name TO FOLDER 'Path' IN NewModule; MOVE FOLDER Module.FolderName TO FOLDER 'Path'; @@ -224,6 +226,11 @@ MOVE MICROFLOW MyModule.ACT_ProcessOrder TO FOLDER 'Orders/Processing'; -- Move entity to different module MOVE ENTITY OldModule.Customer TO NewModule; +-- Java actions and published OData services have no folder clause on CREATE, +-- so MOVE is the only way to place them +MOVE JAVA ACTION MyModule.ODataQuery TO FOLDER 'Support'; +MOVE ODATA SERVICE MyModule.PublicApi TO FOLDER 'Api/Published'; + -- Check impact before cross-module move SHOW IMPACT OF OldModule.CustomerPage; MOVE PAGE OldModule.CustomerPage TO NewModule; diff --git a/cmd/mxcli/syntax/features_security.go b/cmd/mxcli/syntax/features_security.go index 9f6eba6f5..7481baf8b 100644 --- a/cmd/mxcli/syntax/features_security.go +++ b/cmd/mxcli/syntax/features_security.go @@ -21,8 +21,8 @@ func init() { Keywords: []string{ "module role", "create role", "drop role", }, - Syntax: "CREATE MODULE ROLE . [DESCRIPTION ''];\nDROP MODULE ROLE .;", - Example: "CREATE MODULE ROLE Shop.Admin DESCRIPTION 'Full access';\nCREATE MODULE ROLE Shop.User DESCRIPTION 'Read-only access';", + Syntax: "CREATE [OR MODIFY] MODULE ROLE . [DESCRIPTION ''];\nDROP MODULE ROLE .;", + Example: "CREATE MODULE ROLE Shop.Admin DESCRIPTION 'Full access';\n-- OR MODIFY makes a security script re-runnable:\nCREATE OR MODIFY MODULE ROLE Shop.User DESCRIPTION 'Read-only access';", SeeAlso: []string{"security.user-role", "security.entity-access"}, }) diff --git a/docs/01-project/MDL_QUICK_REFERENCE.md b/docs/01-project/MDL_QUICK_REFERENCE.md index ac4e35c32..a4ba27383 100644 --- a/docs/01-project/MDL_QUICK_REFERENCE.md +++ b/docs/01-project/MDL_QUICK_REFERENCE.md @@ -355,7 +355,7 @@ Nested folders use `/` separator: `'Parent/Child/Grandchild'`. Missing folders a | Show demo users | `show demo users;` | Configured demo users | | Show access on element | `show access on microflow\|nanoflow\|page\|entity Mod.Name;` | Which roles can access | | Show security matrix | `show security matrix [in module];` | Full access overview | -| Create module role | `create module role Mod.Role [description 'text'];` | | +| Create module role | `create [or modify] module role Mod.Role [description 'text'];` | `or modify` updates an existing role instead of failing, so a security script can be re-run | | Drop module role | `drop module role Mod.Role;` | | | Create user role | `create user role Name (Mod.Role, ...) [manage all roles];` | Aggregates module roles | | Alter user role | `alter user role Name add\|remove module roles (Mod.Role, ...);` | | diff --git a/mdl-examples/doctype-tests/18-folder-examples.mdl b/mdl-examples/doctype-tests/18-folder-examples.mdl index bc2626cc2..18e5a7cec 100644 --- a/mdl-examples/doctype-tests/18-folder-examples.mdl +++ b/mdl-examples/doctype-tests/18-folder-examples.mdl @@ -89,4 +89,47 @@ drop folder 'Resources' in FolderTest; / -- cleanup +-- ============================================================================ +-- Level 6: Java actions and published OData services +-- ============================================================================ + +/** + * Neither CREATE JAVA ACTION nor CREATE ODATA SERVICE takes a folder clause, so + * MOVE is the only way these documents ever leave the module root. + */ +create java action FolderTest.QueryHelper () returns Boolean as $$ +public class QueryHelper { } +$$; + +move java action FolderTest.QueryHelper to folder 'Support/Java'; + +create non-persistent entity FolderTest.ApiRow ( RowKey: string(60) ); + +CREATE MICROFLOW FolderTest.Read_ApiRows () + RETURNS List of FolderTest.ApiRow AS $Rows +BEGIN + $Rows = CREATE LIST OF FolderTest.ApiRow; + RETURN $Rows; +END; + +create odata service FolderTest.PublicApi ( + path: 'odata/foldertest/', + version: '1.0.0', + ODataVersion: OData4, + namespace: 'FolderTest.PublicApi', + ServiceName: 'PublicApi' +) +{ + publish entity FolderTest.ApiRow as 'ApiRows' ( + ReadMode: microflow FolderTest.Read_ApiRows, + InsertMode: not_supported, + UpdateMode: not_supported, + DeleteMode: not_supported, + Countable: false + ) + expose ( RowKey as 'rowKey' (KEY) ); +}; + +move odata service FolderTest.PublicApi to folder 'Api/Published'; + drop module FolderTest; diff --git a/mdl/ast/ast.go b/mdl/ast/ast.go index dca56fb91..256d2cfd3 100644 --- a/mdl/ast/ast.go +++ b/mdl/ast/ast.go @@ -54,6 +54,8 @@ const ( DocumentTypeEnumeration DocumentType = "ENUMERATION" DocumentTypeConstant DocumentType = "CONSTANT" DocumentTypeDatabaseConnection DocumentType = "DATABASE CONNECTION" + DocumentTypeJavaAction DocumentType = "JAVA ACTION" + DocumentTypeODataService DocumentType = "ODATA SERVICE" ) // MoveStmt represents: MOVE PAGE/MICROFLOW/SNIPPET/NANOFLOW/ENTITY/ENUMERATION Module.Name TO FOLDER 'path' IN Module diff --git a/mdl/ast/ast_microflow.go b/mdl/ast/ast_microflow.go index ac4f48b4d..3f7af75e2 100644 --- a/mdl/ast/ast_microflow.go +++ b/mdl/ast/ast_microflow.go @@ -452,13 +452,18 @@ func (s *CallWebServiceStmt) isMicroflowStatement() {} // ExecuteDatabaseQueryStmt represents: EXECUTE DATABASE QUERY Module.Connection.QueryName ... type ExecuteDatabaseQueryStmt struct { - OutputVariable string // Optional output variable - QueryName string // Full 3-part identifier: Module.Connection.QueryName - DynamicQuery string // Optional dynamic SQL override - Arguments []CallArgument // Parameter mappings (query parameters) - ConnectionArguments []CallArgument // Connection parameter mappings (runtime connection override) - ErrorHandling *ErrorHandlingClause // Optional ON ERROR clause - Annotations *ActivityAnnotations // Optional @position, @caption, @color, @annotation + OutputVariable string // Optional output variable + QueryName string // Full 3-part identifier: Module.Connection.QueryName + DynamicQuery string // Optional dynamic SQL override + // DynamicQueryIsExpression distinguishes `dynamic $Sql` from `dynamic 'SELECT …'`. + // Both reach the executor as a bare string, and the builder has to quote one + // and not the other: quoting an expression sends the literal text `$Sql` to + // the database, which is a syntax error at the far end, not a Mendix one. + DynamicQueryIsExpression bool + Arguments []CallArgument // Parameter mappings (query parameters) + ConnectionArguments []CallArgument // Connection parameter mappings (runtime connection override) + ErrorHandling *ErrorHandlingClause // Optional ON ERROR clause + Annotations *ActivityAnnotations // Optional @position, @caption, @color, @annotation } func (s *ExecuteDatabaseQueryStmt) isMicroflowStatement() {} diff --git a/mdl/ast/ast_odata.go b/mdl/ast/ast_odata.go index 729a8a24a..158129ab7 100644 --- a/mdl/ast/ast_odata.go +++ b/mdl/ast/ast_odata.go @@ -24,7 +24,15 @@ type CreateODataClientStmt struct { UseAuthentication bool HttpUsername string // Mendix expression for username HttpPassword string // Mendix expression for password - ClientCertificate string + + // Whether the credential above was written as a quoted literal rather than + // a constant reference. The visitor strips a literal's quotes, so by the + // time it reaches the executor `'f1api'` and `Module.ApiUser` are both bare + // strings — and only the first is a value mxcli can use for the design-time + // $metadata fetch. A constant is resolved by the runtime, not by us. + HttpUsernameIsLiteral bool + HttpPasswordIsLiteral bool + ClientCertificate string // Microflow references. `ConfigurationMicroflow` (returns // System.ConsumedODataConfiguration) and `HeadersMicroflow` (returns a list @@ -52,6 +60,9 @@ type CreateODataClientStmt struct { type HeaderDef struct { Key string Value string // Mendix expression + // ValueIsLiteral mirrors HttpUsernameIsLiteral: a quoted literal can be sent + // on the design-time fetch, a constant reference cannot. + ValueIsLiteral bool } func (s *CreateODataClientStmt) isStatement() {} diff --git a/mdl/ast/ast_security.go b/mdl/ast/ast_security.go index 09e06cdf2..9c03d99e7 100644 --- a/mdl/ast/ast_security.go +++ b/mdl/ast/ast_security.go @@ -10,6 +10,10 @@ package ast type CreateModuleRoleStmt struct { Name QualifiedName Description string + // CreateOrModify makes the statement idempotent: an existing role has its + // description updated instead of the statement failing, so a security script + // can be re-run. + CreateOrModify bool } func (s *CreateModuleRoleStmt) isStatement() {} diff --git a/mdl/backend/java.go b/mdl/backend/java.go index 73a3e5d72..b906eabb9 100644 --- a/mdl/backend/java.go +++ b/mdl/backend/java.go @@ -12,6 +12,8 @@ import ( type JavaBackend interface { ListJavaActions() ([]*types.JavaAction, error) ListJavaActionsFull() ([]*javaactions.JavaAction, error) + // MoveJavaAction reparents a Java action to an already-updated ContainerID. + MoveJavaAction(ja *javaactions.JavaAction) error ListJavaScriptActions() ([]*types.JavaScriptAction, error) ReadJavaActionByName(qualifiedName string) (*javaactions.JavaAction, error) ReadJavaScriptActionByName(qualifiedName string) (*types.JavaScriptAction, error) diff --git a/mdl/backend/mcp/unsupported_gen.go b/mdl/backend/mcp/unsupported_gen.go index 35878e3b9..637e5e5d6 100644 --- a/mdl/backend/mcp/unsupported_gen.go +++ b/mdl/backend/mcp/unsupported_gen.go @@ -667,13 +667,13 @@ func (unsupportedBackend) ListFolders() (r0 []*types.FolderInfo, err1 error) { return } -func (unsupportedBackend) ListImageCollections() (r0 []*types.ImageCollection, err1 error) { - err1 = errUnsupported("ListImageCollections") +func (unsupportedBackend) ListIconCollections() (r0 []*types.IconCollection, err1 error) { + err1 = errUnsupported("ListIconCollections") return } -func (unsupportedBackend) ListIconCollections() (r0 []*types.IconCollection, err1 error) { - err1 = errUnsupported("ListIconCollections") +func (unsupportedBackend) ListImageCollections() (r0 []*types.ImageCollection, err1 error) { + err1 = errUnsupported("ListImageCollections") return } @@ -827,6 +827,11 @@ func (unsupportedBackend) MoveImportMapping(_ *model.ImportMapping) (err0 error) return } +func (unsupportedBackend) MoveJavaAction(_ *javaactions.JavaAction) (err0 error) { + err0 = errUnsupported("MoveJavaAction") + return +} + func (unsupportedBackend) MoveMicroflow(_ *microflows.Microflow) (err0 error) { err0 = errUnsupported("MoveMicroflow") return @@ -842,6 +847,11 @@ func (unsupportedBackend) MovePage(_ *pages.Page) (err0 error) { return } +func (unsupportedBackend) MovePublishedODataService(_ *model.PublishedODataService) (err0 error) { + err0 = errUnsupported("MovePublishedODataService") + return +} + func (unsupportedBackend) MoveSnippet(_ *pages.Snippet) (err0 error) { err0 = errUnsupported("MoveSnippet") return diff --git a/mdl/backend/mock/backend.go b/mdl/backend/mock/backend.go index 7930593ef..b2d9e8c62 100644 --- a/mdl/backend/mock/backend.go +++ b/mdl/backend/mock/backend.go @@ -185,6 +185,8 @@ type MockBackend struct { CreateDatabaseConnectionFunc func(conn *model.DatabaseConnection) error UpdateDatabaseConnectionFunc func(conn *model.DatabaseConnection) error MoveDatabaseConnectionFunc func(conn *model.DatabaseConnection) error + MoveJavaActionFunc func(ja *javaactions.JavaAction) error + MovePublishedODataServiceFunc func(svc *model.PublishedODataService) error DeleteDatabaseConnectionFunc func(id model.ID) error ListDataTransformersFunc func() ([]*model.DataTransformer, error) CreateDataTransformerFunc func(dt *model.DataTransformer) error diff --git a/mdl/backend/mock/mock_service.go b/mdl/backend/mock/mock_service.go index 6dbf0cfd1..cb37e74fd 100644 --- a/mdl/backend/mock/mock_service.go +++ b/mdl/backend/mock/mock_service.go @@ -2,7 +2,12 @@ package mock -import "github.com/mendixlabs/mxcli/model" +import ( + "errors" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/javaactions" +) func (m *MockBackend) ListConsumedODataServices() ([]*model.ConsumedODataService, error) { if m.ListConsumedODataServicesFunc != nil { @@ -172,6 +177,20 @@ func (m *MockBackend) MoveDatabaseConnection(conn *model.DatabaseConnection) err return nil } +func (m *MockBackend) MoveJavaAction(ja *javaactions.JavaAction) error { + if m.MoveJavaActionFunc != nil { + return m.MoveJavaActionFunc(ja) + } + return errors.New("MockBackend.MoveJavaAction not configured") +} + +func (m *MockBackend) MovePublishedODataService(svc *model.PublishedODataService) error { + if m.MovePublishedODataServiceFunc != nil { + return m.MovePublishedODataServiceFunc(svc) + } + return errors.New("MockBackend.MovePublishedODataService not configured") +} + func (m *MockBackend) DeleteDatabaseConnection(id model.ID) error { if m.DeleteDatabaseConnectionFunc != nil { return m.DeleteDatabaseConnectionFunc(id) diff --git a/mdl/backend/modelsdk/domainmodel.go b/mdl/backend/modelsdk/domainmodel.go index a8d61fa80..23260e7e7 100644 --- a/mdl/backend/modelsdk/domainmodel.go +++ b/mdl/backend/modelsdk/domainmodel.go @@ -266,6 +266,24 @@ func attributeFromGen(a *genDm.Attribute) *domainmodel.Attribute { // View-entity attribute: the OQL column reference must survive a // read-modify-write (e.g. MOVE ENTITY) or the view goes out of sync (CE6770). attr.Value = &domainmodel.AttributeValue{ViewReference: v.Reference()} + case *genRest.ODataMappedValue: + // External-entity attribute: the mapping to the remote OData property. + // Reading it back is what makes a read-modify-write safe — without it + // every attribute of an external entity comes back unmapped, and the + // writer's `isExternal && a.RemoteName != ""` arm falls through to a + // plain StoredValue. The entity then no longer matches the contract: + // "Attribute 'year' of external entity 'Stg_Season' is not supported." + attr.RemoteName = v.RemoteName() + attr.RemoteType = v.RemoteType() + attr.Filterable = v.Filterable() + attr.Sortable = v.Sortable() + attr.Creatable = v.Creatable() + attr.Updatable = v.Updatable() + case *genRest.ODataMappedPrimitiveCollectionValue: + // The single attribute of a primitive-collection NPE (issue #718). + attr.RemoteName = v.RemoteName() + attr.RemoteType = v.RemoteType() + attr.IsPrimitiveCollection = true } return attr } diff --git a/mdl/backend/modelsdk/external_entity_read_test.go b/mdl/backend/modelsdk/external_entity_read_test.go index 556773014..3c737c9fb 100644 --- a/mdl/backend/modelsdk/external_entity_read_test.go +++ b/mdl/backend/modelsdk/external_entity_read_test.go @@ -227,3 +227,49 @@ func TestExternalEntity_PrimitiveCollectionSourceRoundTrip(t *testing.T) { t.Errorf("RemoteServiceName = %q", got.RemoteServiceName) } } + +// TestExternalEntity_AttributeRemoteMappingRoundTrip is the attribute-level half +// of #782, found by mxcli-formula1 #25: entityFromGen learned to read the +// entity's own remote fields, but attributeFromGen still handled only +// StoredValue and OqlViewValue. A Rest$ODataMappedValue therefore came back with +// no RemoteName, and the write path's `isExternal && a.RemoteName != ""` arm fell +// through to a plain StoredValue on the next read-modify-write. +// +// The visible failure is a `create or modify external entity` that touches only +// an entity-level property and detonates every attribute: +// +// [CE6612] "Attribute 'circuitId' of external entity 'Stg_Circuit' is not supported." +// +// one per attribute. Confirmed on 11.12.1 against a real contract import: three +// CE6612 before the fix, none after. +func TestExternalEntity_AttributeRemoteMappingRoundTrip(t *testing.T) { + proj, modID := externalEntityFixture(t, func(e *domainmodel.Entity) { + e.Attributes = []*domainmodel.Attribute{{ + Name: "ProductName", + Type: &domainmodel.StringAttributeType{Length: 120}, + RemoteName: "Name", + RemoteType: "Edm.String", + Filterable: true, + Sortable: true, + Updatable: true, + }} + }) + got := readEntity(t, proj, modID, "Products") + + if len(got.Attributes) != 1 { + t.Fatalf("got %d attributes, want 1", len(got.Attributes)) + } + a := got.Attributes[0] + if a.RemoteName != "Name" { + t.Errorf("RemoteName = %q, want Name — the OData mapping did not survive the read", a.RemoteName) + } + if a.RemoteType != "Edm.String" { + t.Errorf("RemoteType = %q, want Edm.String", a.RemoteType) + } + // The per-attribute capability flags live on the same ODataMappedValue and + // are equally lost if the case arm is missing. + if !a.Filterable || !a.Sortable || !a.Updatable { + t.Errorf("capability flags lost: filterable=%v sortable=%v updatable=%v", + a.Filterable, a.Sortable, a.Updatable) + } +} diff --git a/mdl/backend/modelsdk/move_documents_write.go b/mdl/backend/modelsdk/move_documents_write.go index b93afa3cf..9919b7f03 100644 --- a/mdl/backend/modelsdk/move_documents_write.go +++ b/mdl/backend/modelsdk/move_documents_write.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/javaactions" "github.com/mendixlabs/mxcli/sdk/microflows" "github.com/mendixlabs/mxcli/sdk/pages" ) @@ -69,3 +70,17 @@ func (b *Backend) MoveDatabaseConnection(conn *model.DatabaseConnection) error { } return b.moveUnit(conn.ID, conn.ContainerID, "DatabaseConnection") } + +func (b *Backend) MoveJavaAction(ja *javaactions.JavaAction) error { + if ja == nil { + return fmt.Errorf("MoveJavaAction: nil java action") + } + return b.moveUnit(ja.ID, ja.ContainerID, "JavaAction") +} + +func (b *Backend) MovePublishedODataService(svc *model.PublishedODataService) error { + if svc == nil { + return fmt.Errorf("MovePublishedODataService: nil service") + } + return b.moveUnit(svc.ID, svc.ContainerID, "PublishedODataService") +} diff --git a/mdl/backend/modelsdk/odata_write.go b/mdl/backend/modelsdk/odata_write.go index 7a153da21..a02b7d92b 100644 --- a/mdl/backend/modelsdk/odata_write.go +++ b/mdl/backend/modelsdk/odata_write.go @@ -354,7 +354,7 @@ func publishedMemberToGen(m *model.PublishedMember, ownerQN string) element.Elem addBool(g, "Filterable", m.Filterable) addBool(g, "Sortable", m.Sortable) addBool(g, "IsPartOfKey", m.IsPartOfKey) - addBool(g, "EnumerationAsString", false) + addBool(g, "EnumerationAsString", m.EnumerationAsString) addBool(g, "StringAsGuid", false) return g } diff --git a/mdl/backend/modelsdk/unimplemented_gen.go b/mdl/backend/modelsdk/unimplemented_gen.go index c0a682247..ce24b4208 100644 --- a/mdl/backend/modelsdk/unimplemented_gen.go +++ b/mdl/backend/modelsdk/unimplemented_gen.go @@ -595,6 +595,11 @@ func (unimplemented) ListFolders() ([]*types.FolderInfo, error) { return r0, errUnimplemented("ListFolders") } +func (unimplemented) ListIconCollections() ([]*types.IconCollection, error) { + var r0 []*types.IconCollection + return r0, errUnimplemented("ListIconCollections") +} + func (unimplemented) ListImageCollections() ([]*types.ImageCollection, error) { var r0 []*types.ImageCollection return r0, errUnimplemented("ListImageCollections") @@ -744,6 +749,10 @@ func (unimplemented) MoveImportMapping(_ *model.ImportMapping) error { return errUnimplemented("MoveImportMapping") } +func (unimplemented) MoveJavaAction(_ *javaactions.JavaAction) error { + return errUnimplemented("MoveJavaAction") +} + func (unimplemented) MoveMicroflow(_ *microflows.Microflow) error { return errUnimplemented("MoveMicroflow") } @@ -756,6 +765,10 @@ func (unimplemented) MovePage(_ *pages.Page) error { return errUnimplemented("MovePage") } +func (unimplemented) MovePublishedODataService(_ *model.PublishedODataService) error { + return errUnimplemented("MovePublishedODataService") +} + func (unimplemented) MoveSnippet(_ *pages.Snippet) error { return errUnimplemented("MoveSnippet") } diff --git a/mdl/backend/mpr/backend.go b/mdl/backend/mpr/backend.go index fe3c68050..4daeaa705 100644 --- a/mdl/backend/mpr/backend.go +++ b/mdl/backend/mpr/backend.go @@ -6,6 +6,8 @@ package mprbackend import ( + "errors" + "github.com/mendixlabs/mxcli/mdl/backend" "github.com/mendixlabs/mxcli/mdl/linter" "github.com/mendixlabs/mxcli/mdl/types" @@ -501,6 +503,18 @@ func (b *MprBackend) UpdateDatabaseConnection(conn *model.DatabaseConnection) er func (b *MprBackend) MoveDatabaseConnection(conn *model.DatabaseConnection) error { return b.writer.MoveDatabaseConnection(conn) } +func (b *MprBackend) MoveJavaAction(ja *javaactions.JavaAction) error { + if ja == nil { + return errors.New("MoveJavaAction: nil java action") + } + return b.writer.MoveUnitByID(string(ja.ID), string(ja.ContainerID)) +} +func (b *MprBackend) MovePublishedODataService(svc *model.PublishedODataService) error { + if svc == nil { + return errors.New("MovePublishedODataService: nil service") + } + return b.writer.MoveUnitByID(string(svc.ID), string(svc.ContainerID)) +} func (b *MprBackend) DeleteDatabaseConnection(id model.ID) error { return b.writer.DeleteDatabaseConnection(id) } diff --git a/mdl/backend/service.go b/mdl/backend/service.go index 2d1403064..2b61be15c 100644 --- a/mdl/backend/service.go +++ b/mdl/backend/service.go @@ -25,6 +25,9 @@ type ODataBackend interface { DeleteConsumedODataService(id model.ID) error CreatePublishedODataService(svc *model.PublishedODataService) error UpdatePublishedODataService(svc *model.PublishedODataService) error + // MovePublishedODataService reparents the service document to an + // already-updated ContainerID, leaving its contents alone. + MovePublishedODataService(svc *model.PublishedODataService) error DeletePublishedODataService(id model.ID) error } diff --git a/mdl/executor/cmd_contract.go b/mdl/executor/cmd_contract.go index 0d6357cd7..9fb38eec5 100644 --- a/mdl/executor/cmd_contract.go +++ b/mdl/executor/cmd_contract.go @@ -444,13 +444,18 @@ func edmToMendixType(p *types.EdmProperty) string { // reservedEntityAttrNames are Mendix-reserved attribute names that must be // renamed when imported from an OData property of the same name. -// These names conflict with Mendix system members or runtime internals. // The check is case-insensitive (see attrNameForOData). +// +// Every entry is one Mendix rejects with CE7247 "The name 'x' is a reserved +// word." Verified on 11.12.1 by importing a contract with a property for each +// name and prefixing disabled: seven errors, one per name below. `name` was on +// this list and is NOT among them — an external entity with an attribute +// literally named `name` builds clean, and prefixing it mangled the commonest +// property in any contract (mxcli-formula1 #28). Do not add a name here without +// a CE7247 to point at. var reservedEntityAttrNames = map[string]bool{ // Mendix internal identifier "id": true, - // Mendix system-managed attribute for the object name (present on many entities) - "name": true, // System ownership association (HasOwner / System.owner) "owner": true, // System audit associations (HasChangedBy / System.changedBy) @@ -523,6 +528,10 @@ func createExternalEntities(ctx *ExecContext, s *ast.CreateExternalEntitiesStmt) serviceRef := s.ServiceRef.String() var created, updated, skipped, failed int + // Attribute names the import had to change because Mendix reserves them. + // Reported at the end so the local name never silently diverges from the + // contract; the mapping still points at the remote property either way. + var renamed []string for _, schema := range doc.Schemas { for _, et := range schema.EntityTypes { @@ -598,6 +607,13 @@ func createExternalEntities(ctx *ExecContext, s *ast.CreateExternalEntitiesStmt) } nonInsertable := make(map[string]bool) nonUpdatable := make(map[string]bool) + // Filter/Sort restrictions name the properties the service refuses to + // filter or sort on. Marking them filterable anyway is CE6630 + // ("'latitude' is marked Filterable=False in the OData service, but + // True in the app") — one per property, on an import whose whole job + // is to match the contract. + nonFilterable := make(map[string]bool) + nonSortable := make(map[string]bool) if entitySet != nil { for _, name := range entitySet.NonInsertableProperties { nonInsertable[name] = true @@ -605,6 +621,12 @@ func createExternalEntities(ctx *ExecContext, s *ast.CreateExternalEntitiesStmt) for _, name := range entitySet.NonUpdatableProperties { nonUpdatable[name] = true } + for _, name := range entitySet.NonFilterableProperties { + nonFilterable[name] = true + } + for _, name := range entitySet.NonSortableProperties { + nonSortable[name] = true + } } // Build attributes from merged properties @@ -636,13 +658,19 @@ func createExternalEntities(ctx *ExecContext, s *ast.CreateExternalEntitiesStmt) } attrName := attrNameForOData(p.Name, et.Name) + if attrName != p.Name { + // Never let a rename be discovered later, when a page written + // against the published $metadata fails with "The selected + // attribute … no longer exists" (mxcli-formula1 #28). + renamed = append(renamed, fmt.Sprintf("%s.%s: %s -> %s", mendixName, p.Name, p.Name, attrName)) + } attr := &domainmodel.Attribute{ Name: attrName, Type: edmToDomainModelAttrType(p, keyPropSet[p.Name]), RemoteName: p.Name, RemoteType: p.Type, - Filterable: true, - Sortable: true, + Filterable: !nonFilterable[p.Name], + Sortable: !nonSortable[p.Name], Creatable: creatable, Updatable: updatable, } @@ -719,6 +747,15 @@ func createExternalEntities(ctx *ExecContext, s *ast.CreateExternalEntitiesStmt) fmt.Fprintf(ctx.Output, "\nFrom %s into %s: %d created, %d updated, %d skipped, %d failed\n", svcQN, targetModule, created, updated, skipped, failed) + if len(renamed) > 0 { + sort.Strings(renamed) + fmt.Fprintf(ctx.Output, "\n %d attribute name(s) changed — Mendix reserves the contract's spelling (CE7247),\n", len(renamed)) + fmt.Fprintf(ctx.Output, " so a page or expression must use the local name, not the one in $metadata:\n") + for _, r := range renamed { + fmt.Fprintf(ctx.Output, " %s\n", r) + } + } + return nil } @@ -1157,7 +1194,12 @@ func applyExternalEntityFields( ent.Source = "Rest$ODataRemoteEntitySource" ent.Persistable = true ent.RemoteEntitySet = entitySet.Name - ent.Countable = true + // Countable follows the contract when it says so. OData's own default is + // countable, so an unannotated set stays true — but a service that + // declares CountRestrictions/Countable=false and an app that says true is + // CE6630 ("marked Countable=False in the OData service, but True in the + // app"), which is the whole point of generating from $metadata. + ent.Countable = entitySet.Countable == nil || *entitySet.Countable // Capabilities default to false (Mendix's conservative read-only default) // when the entity set has no Insert/Delete restriction annotation — an // unannotated service is treated as read-only, and the app must match or diff --git a/mdl/executor/cmd_contract_reserved_test.go b/mdl/executor/cmd_contract_reserved_test.go new file mode 100644 index 000000000..bed6350da --- /dev/null +++ b/mdl/executor/cmd_contract_reserved_test.go @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import "testing" + +// mxcli-formula1 findings #28: an attribute named `name` came out of +// CREATE EXTERNAL ENTITIES prefixed with the remote type — `Stg_Drivername`, +// `Circuitname` — so a page written against the published $metadata failed with +// "The selected attribute 'F1Live.Drivers.name' no longer exists", and the same +// field carried a different name in every module because the remote type names +// differ. +// +// `name` was simply not reserved. Adjudicated on 11.12.1 by importing a contract +// with a property for every name on the list, prefixing disabled: Mendix +// answered CE7247 for seven of the eight and said nothing about `name`. +func TestAttrNameForOData(t *testing.T) { + // Every one of these is a CE7247 "The name 'x' is a reserved word." + for _, reserved := range []string{"id", "owner", "changedBy", "changedDate", "createdDate", "type", "context"} { + if got := attrNameForOData(reserved, "Driver"); got != "Driver"+reserved { + t.Errorf("attrNameForOData(%q) = %q, want it disambiguated — Mendix rejects the bare name with CE7247", reserved, got) + } + } + + // `name` is an ordinary attribute name and must survive untouched. So must + // anything else the contract happens to call a property. + for _, ok := range []string{"name", "driverRef", "surname", "nationality", "label"} { + if got := attrNameForOData(ok, "Driver"); got != ok { + t.Errorf("attrNameForOData(%q) = %q, want it unchanged — Mendix accepts it", ok, got) + } + } +} + +// The check is case-insensitive: a contract using Id or TYPE hits the same +// reserved word. +func TestAttrNameForOData_CaseInsensitive(t *testing.T) { + for _, v := range []string{"Id", "ID", "TYPE", "Owner"} { + if got := attrNameForOData(v, "Thing"); got == v { + t.Errorf("attrNameForOData(%q) left it unchanged; reserved words are case-insensitive", v) + } + } + // …but a name that merely contains one is fine. + for _, v := range []string{"identifier", "typeCode", "ownerName"} { + if got := attrNameForOData(v, "Thing"); got != v { + t.Errorf("attrNameForOData(%q) = %q, want unchanged — it is not the reserved word itself", v, got) + } + } +} diff --git a/mdl/executor/cmd_contract_test.go b/mdl/executor/cmd_contract_test.go index 696142eb9..2a325c626 100644 --- a/mdl/executor/cmd_contract_test.go +++ b/mdl/executor/cmd_contract_test.go @@ -23,8 +23,6 @@ func TestAttrNameForOData_ReservedWords(t *testing.T) { // Already-covered names {"Id", "Photo", "PhotoId"}, {"id", "Photo", "Photoid"}, - {"Name", "Airline", "AirlineName"}, - {"name", "Airline", "Airlinename"}, // Newly-added reserved names (issue #526) {"Owner", "Trip", "TripOwner"}, {"owner", "Trip", "Tripowner"}, @@ -38,7 +36,13 @@ func TestAttrNameForOData_ReservedWords(t *testing.T) { {"changeddate", "Event", "Eventchangeddate"}, {"CreatedDate", "Event", "EventCreatedDate"}, {"createddate", "Event", "Eventcreateddate"}, - // Non-reserved names must pass through unchanged + // Non-reserved names must pass through unchanged. `name` belongs here: + // it was on the reserved list and is not reserved — verified on 11.12.1 + // by importing a contract with a property per listed name and prefixing + // disabled, which produced CE7247 for every other name and nothing for + // this one (mxcli-formula1 #28). + {"Name", "Airline", "Name"}, + {"name", "Airline", "name"}, {"AirlineCode", "Airline", "AirlineCode"}, {"Concurrency", "Airline", "Concurrency"}, {"FirstName", "Person", "FirstName"}, @@ -135,8 +139,13 @@ func TestCreateNavigationAssociations_NoDuplicateOnReimport(t *testing.T) { // TestMendixAttrTypeToEdm guards the Mendix→EDM type mapping used to populate a // published attribute's EdmType. Without it Studio Pro reports CE5016 -// ("published as ."). String/Decimal/Boolean/DateTimeOffset are verified against -// Studio Pro's corrected BSON. +// ("published as ."). +// +// Every row is now adjudicated by mxbuild rather than assumed: one attribute of +// each type published in one service on 11.12.1, then the CE5016s read off the +// build. This test previously pinned Integer to Edm.Int32, which was an +// unverified guess and wrong — Mendix publishes Integer as Int64, so every whole +// number in a published service failed the build (mxcli-formula1 #16). func TestMendixAttrTypeToEdm(t *testing.T) { cases := []struct { typ domainmodel.AttributeType @@ -144,13 +153,14 @@ func TestMendixAttrTypeToEdm(t *testing.T) { }{ {&domainmodel.StringAttributeType{}, "Edm.String"}, {&domainmodel.HashedStringAttributeType{}, "Edm.String"}, - {&domainmodel.IntegerAttributeType{}, "Edm.Int32"}, + {&domainmodel.IntegerAttributeType{}, "Edm.Int64"}, {&domainmodel.LongAttributeType{}, "Edm.Int64"}, {&domainmodel.AutoNumberAttributeType{}, "Edm.Int64"}, {&domainmodel.DecimalAttributeType{}, "Edm.Decimal"}, {&domainmodel.BooleanAttributeType{}, "Edm.Boolean"}, {&domainmodel.DateTimeAttributeType{}, "Edm.DateTimeOffset"}, - {&domainmodel.BinaryAttributeType{}, "Edm.Binary"}, + {&domainmodel.BinaryAttributeType{}, "Edm.Binary"}, // never reachable: CE5013 forbids exposing Binary at all + {&domainmodel.EnumerationAttributeType{}, "Edm.String"}, {nil, ""}, } for _, c := range cases { diff --git a/mdl/executor/cmd_microflows_builder_calls.go b/mdl/executor/cmd_microflows_builder_calls.go index 380378585..cc67c7240 100644 --- a/mdl/executor/cmd_microflows_builder_calls.go +++ b/mdl/executor/cmd_microflows_builder_calls.go @@ -1344,13 +1344,28 @@ func buildRestParameterMappings( return pathMappings, queryMappings } +// dynamicQueryExpression renders the statement's dynamic query as the Mendix +// expression the action stores. +// +// A literal SQL string has to be quoted, because the field holds an expression — +// but an expression must be passed through untouched. Quoting `$Sql` sends the +// four characters "$Sql" to the database: +// +// ERROR - ExternalDatabaseConnector: Parser Error: syntax error at or near "$" +// +// which blocks runtime-built SQL entirely. The two spellings are only +// distinguishable at parse time, hence DynamicQueryIsExpression. +func dynamicQueryExpression(s *ast.ExecuteDatabaseQueryStmt) string { + q := s.DynamicQuery + if q == "" || s.DynamicQueryIsExpression || strings.HasPrefix(q, "'") { + return q + } + return "'" + strings.ReplaceAll(q, "'", "''") + "'" +} + // addExecuteDatabaseQueryAction creates an EXECUTE DATABASE QUERY statement. func (fb *flowBuilder) addExecuteDatabaseQueryAction(s *ast.ExecuteDatabaseQueryStmt) model.ID { - // DynamicQuery is a Mendix expression — string literals need single quotes - dynamicQuery := s.DynamicQuery - if dynamicQuery != "" && !strings.HasPrefix(dynamicQuery, "'") { - dynamicQuery = "'" + strings.ReplaceAll(dynamicQuery, "'", "''") + "'" - } + dynamicQuery := dynamicQueryExpression(s) action := µflows.ExecuteDatabaseQueryAction{ BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, diff --git a/mdl/executor/cmd_microflows_dynamic_query_test.go b/mdl/executor/cmd_microflows_dynamic_query_test.go new file mode 100644 index 000000000..f3d2723d6 --- /dev/null +++ b/mdl/executor/cmd_microflows_dynamic_query_test.go @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// mxcli-formula1 findings #21: `execute database query … dynamic $Sql` reached +// the runtime as the string literal '$Sql', so DuckDB was asked to execute the +// four characters: +// +// ERROR - ExternalDatabaseConnector: Parser Error: syntax error at or near "$" +// +// The builder quoted anything not already starting with a quote, and the AST +// kept no literal-vs-expression flag, so it could not tell them apart. This +// blocked runtime-built SQL — query pushdown — outright. +func TestDynamicQueryExpressionIsNotQuoted(t *testing.T) { + cases := []struct { + name string + stmt *ast.ExecuteDatabaseQueryStmt + want string + }{ + { + "a variable passes through untouched", + &ast.ExecuteDatabaseQueryStmt{DynamicQuery: "$Sql", DynamicQueryIsExpression: true}, + "$Sql", + }, + { + "so does a built expression", + &ast.ExecuteDatabaseQueryStmt{DynamicQuery: "'SELECT * FROM t LIMIT ' + toString($Limit)", DynamicQueryIsExpression: true}, + "'SELECT * FROM t LIMIT ' + toString($Limit)", + }, + { + "a literal is still quoted, because the field holds an expression", + &ast.ExecuteDatabaseQueryStmt{DynamicQuery: "SELECT * FROM t"}, + "'SELECT * FROM t'", + }, + { + "a literal's own quotes are doubled, Mendix-style", + &ast.ExecuteDatabaseQueryStmt{DynamicQuery: "SELECT 'a'"}, + "'SELECT ''a'''", + }, + { + "no dynamic query stays empty rather than becoming two quotes", + &ast.ExecuteDatabaseQueryStmt{}, + "", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := dynamicQueryExpression(tc.stmt); got != tc.want { + t.Errorf("got %q, want %q", got, tc.want) + } + }) + } +} diff --git a/mdl/executor/cmd_move.go b/mdl/executor/cmd_move.go index 28e2d408d..ff5034c0c 100644 --- a/mdl/executor/cmd_move.go +++ b/mdl/executor/cmd_move.go @@ -81,6 +81,14 @@ func execMove(ctx *ExecContext, s *ast.MoveStmt) error { if err := moveDatabaseConnection(ctx, s.Name, targetContainerID); err != nil { return err } + case ast.DocumentTypeJavaAction: + if err := moveJavaAction(ctx, s.Name, targetContainerID); err != nil { + return err + } + case ast.DocumentTypeODataService: + if err := movePublishedODataService(ctx, s.Name, targetContainerID); err != nil { + return err + } default: return mdlerrors.NewUnsupported("unsupported document type: " + string(s.DocumentType)) } @@ -388,3 +396,61 @@ func moveDatabaseConnection(ctx *ExecContext, name ast.QualifiedName, targetCont return mdlerrors.NewNotFound("database connection", name.String()) } + +// moveJavaAction moves a Java action to a new container. +// +// Java actions and published OData services had no MOVE doctype at all, and +// neither CREATE form takes a folder clause — so those documents could never +// leave the module root from MDL (mxcli-formula1 #32). +func moveJavaAction(ctx *ExecContext, name ast.QualifiedName, targetContainerID model.ID) error { + actions, err := ctx.Backend.ListJavaActionsFull() + if err != nil { + return mdlerrors.NewBackend("list java actions", err) + } + + h, err := getHierarchy(ctx) + if err != nil { + return mdlerrors.NewBackend("build hierarchy", err) + } + + for _, ja := range actions { + modID := h.FindModuleID(ja.ContainerID) + if h.GetModuleName(modID) == name.Module && ja.Name == name.Name { + ja.ContainerID = targetContainerID + if err := ctx.Backend.MoveJavaAction(ja); err != nil { + return mdlerrors.NewBackend("move java action", err) + } + fmt.Fprintf(ctx.Output, "Moved java action %s to new location\n", name.String()) + return nil + } + } + + return mdlerrors.NewNotFound("java action", name.String()) +} + +// movePublishedODataService moves a published OData service to a new container. +func movePublishedODataService(ctx *ExecContext, name ast.QualifiedName, targetContainerID model.ID) error { + services, err := ctx.Backend.ListPublishedODataServices() + if err != nil { + return mdlerrors.NewBackend("list published OData services", err) + } + + h, err := getHierarchy(ctx) + if err != nil { + return mdlerrors.NewBackend("build hierarchy", err) + } + + for _, svc := range services { + modID := h.FindModuleID(svc.ContainerID) + if h.GetModuleName(modID) == name.Module && svc.Name == name.Name { + svc.ContainerID = targetContainerID + if err := ctx.Backend.MovePublishedODataService(svc); err != nil { + return mdlerrors.NewBackend("move published OData service", err) + } + fmt.Fprintf(ctx.Output, "Moved odata service %s to new location\n", name.String()) + return nil + } + } + + return mdlerrors.NewNotFound("odata service", name.String()) +} diff --git a/mdl/executor/cmd_odata.go b/mdl/executor/cmd_odata.go index 28a0a1d89..2e98d8cf9 100644 --- a/mdl/executor/cmd_odata.go +++ b/mdl/executor/cmd_odata.go @@ -1127,9 +1127,15 @@ Got: %s`, stmt.ServiceUrl) } newSvc.MetadataUrl = normalizedUrl - metadata, hash, err := fetchODataMetadata(normalizedUrl) + auth := metadataAuthFromStmt(stmt) + metadata, hash, err := fetchODataMetadata(normalizedUrl, auth) if err != nil { fmt.Fprintf(ctx.Output, "Warning: could not fetch $metadata: %v\n", err) + for _, hint := range auth.hints() { + fmt.Fprintf(ctx.Output, " %s\n", hint) + } + fmt.Fprintf(ctx.Output, " The client is created with no cached entity types, so a following\n") + fmt.Fprintf(ctx.Output, " 'create external entities from %s.%s' will import nothing.\n", stmt.Name.Module, stmt.Name.Name) } else if metadata != "" { newSvc.Metadata = metadata newSvc.MetadataHash = hash @@ -1333,6 +1339,8 @@ func createODataService(ctx *ExecContext, stmt *ast.CreateODataServiceStmt) erro modName := h.GetModuleName(modID) if strings.EqualFold(modName, stmt.Name.Module) && strings.EqualFold(svc.Name, stmt.Name.Name) { if stmt.CreateOrModify { + // Snapshot the grants before anything below can clear them. + existingRoles := append([]string(nil), svc.AllowedModuleRoles...) svc.Documentation = stmt.Documentation if stmt.Path != "" { svc.Path = stmt.Path @@ -1365,6 +1373,38 @@ func createODataService(ctx *ExecContext, stmt *ast.CreateODataServiceStmt) erro if len(stmt.AuthenticationTypes) > 0 { svc.AuthenticationTypes = stmt.AuthenticationTypes } + // Published entities are replaced wholesale when the statement + // supplies any. Previously the modify branch ignored them + // entirely: editing a `publish entity` block and re-running + // left the served $metadata unchanged, so `Filterable` or + // `Countable` changes appeared to do nothing and the only + // thing that worked was drop + create. Replacing rather than + // merging is what makes the script the description of the + // service — a member removed from the script is removed from + // the service, which merging could never express. + if len(stmt.Entities) > 0 { + svc.EntityTypes = nil + svc.EntitySets = nil + for _, entityDef := range stmt.Entities { + entityType, entitySet := astEntityDefToModel(ctx, entityDef) + svc.EntityTypes = append(svc.EntityTypes, entityType) + svc.EntitySets = append(svc.EntitySets, entitySet) + } + } + // AllowedModuleRoles is granted by a separate statement + // (`grant access on odata service …`) and cannot be expressed + // here, so a modify must carry it through or the build fails + // with "At least one allowed role must be selected for the + // published OData service to be accessible." + // + // A guard, not a fix for an observed defect: the loss was + // reported (mxcli-formula1 #26) but did not reproduce on + // 11.12.1 — grants survived a modify on both the current and + // the previous build. Kept because the invariant is real and + // the cost is a slice copy; if it never fires, nothing is lost. + if len(svc.AllowedModuleRoles) == 0 && len(existingRoles) > 0 { + svc.AllowedModuleRoles = existingRoles + } if err := ctx.Backend.UpdatePublishedODataService(svc); err != nil { return mdlerrors.NewBackend("update OData service", err) } @@ -1620,8 +1660,12 @@ type assocMembership struct { // return empty collections rather than failing the whole publish. // mendixAttrTypeToEdm maps a Mendix attribute type to the OData EDM type Studio // Pro publishes it as (the PublishedAttribute.EdmType field). Inverse of -// edmToDomainModelAttrType. String/Decimal/Boolean/DateTimeOffset verified -// against Studio Pro output; the rest follow the standard OData mapping. +// edmToDomainModelAttrType. +// +// Every case here has been adjudicated by mxbuild rather than assumed: an +// attribute published as the wrong EDM type is CE5016 ("has type Integer, but is +// published as Edm.Int32"), one error per attribute. Verified on 11.12.1 by +// publishing one attribute of each type and reading the errors. func mendixAttrTypeToEdm(t domainmodel.AttributeType) string { if t == nil { return "" @@ -1629,9 +1673,10 @@ func mendixAttrTypeToEdm(t domainmodel.AttributeType) string { switch t.GetTypeName() { case "String", "HashedString": return "Edm.String" - case "Integer": - return "Edm.Int32" - case "Long", "AutoNumber": + case "Integer", "Long", "AutoNumber": + // Mendix publishes Integer as Int64 too, not Int32 — an Integer is + // 64-bit in the Mendix type system, and Int32 is CE5016 on every whole + // number in the service. return "Edm.Int64" case "Decimal": return "Edm.Decimal" @@ -1640,18 +1685,38 @@ func mendixAttrTypeToEdm(t domainmodel.AttributeType) string { case "DateTime", "Date": return "Edm.DateTimeOffset" case "Binary": + // Kept so the mapping is total, but a Binary attribute cannot actually + // be exposed over OData — Mendix rejects it outright with CE5013 + // regardless of the published type. return "Edm.Binary" case "Enumeration": - // Enums exposed as string (EnumerationAsString path). A non-string enum - // exposure would use the enum's own type — not yet modelled. + // Paired with EnumerationAsString=true (see enumPublishedAsString). + // Edm.String with that flag false is rejected: Mendix then wants the + // enumeration published in the service and typed as its own EDM enum. return "Edm.String" default: return "Edm.String" } } -func lookupEntityMembers(ctx *ExecContext, entityQN ast.QualifiedName) (map[string]string, map[string]*assocMembership) { - attrs := make(map[string]string) // attr name -> OData EDM type (for the published EdmType) +// publishedAttrType is how one Mendix attribute publishes over OData: the EDM +// type, plus whether it is an enumeration flattened to a string. The two travel +// together because Edm.String alone is ambiguous — a String and an +// EnumerationAsString enum both carry it, and only the flag tells them apart. +type publishedAttrType struct { + Edm string + AsString bool +} + +// enumPublishedAsString reports whether a published attribute needs the +// EnumerationAsString flag — i.e. whether it is an enumeration at all. The flag +// and the Edm.String type are a matched pair; see mendixAttrTypeToEdm. +func enumPublishedAsString(t domainmodel.AttributeType) bool { + return t != nil && t.GetTypeName() == "Enumeration" +} + +func lookupEntityMembers(ctx *ExecContext, entityQN ast.QualifiedName) (map[string]publishedAttrType, map[string]*assocMembership) { + attrs := make(map[string]publishedAttrType) // attr name -> how it publishes assocs := make(map[string]*assocMembership) if ctx == nil || ctx.Backend == nil { return attrs, assocs @@ -1676,7 +1741,10 @@ func lookupEntityMembers(ctx *ExecContext, entityQN ast.QualifiedName) (map[stri } if thisEntity != nil { for _, a := range thisEntity.Attributes { - attrs[a.Name] = mendixAttrTypeToEdm(a.Type) + attrs[a.Name] = publishedAttrType{ + Edm: mendixAttrTypeToEdm(a.Type), + AsString: enumPublishedAsString(a.Type), + } } } for _, a := range dm.Associations { @@ -1741,9 +1809,10 @@ func astEntityDefToModel(ctx *ExecContext, def *ast.PublishedEntityDef) (*model. member.ExposedName = member.Name } // Auto-detect kind: attribute first, association as fallback. - if edmType, ok := entityAttrs[m.Name]; ok { + if pub, ok := entityAttrs[m.Name]; ok { member.Kind = "attribute" - member.EdmType = edmType + member.EdmType = pub.Edm + member.EnumerationAsString = pub.AsString } else if assoc := moduleAssocs[m.Name]; assoc != nil { member.Kind = "association" member.ExposedAssociationName = m.Name @@ -1796,7 +1865,82 @@ func astEntityDefToModel(ctx *ExecContext, def *ast.PublishedEntityDef) (*model. // Returns the metadata XML and its SHA-256 hash, or empty strings if the fetch fails. // Note: metadataUrl is expected to be already normalized by NormalizeURL() in createODataClient, // so all relative paths have been converted to absolute file:// URLs. -func fetchODataMetadata(metadataUrl string) (metadata string, hash string, err error) { +// metadataFetchAuth carries the credentials and headers used for the +// design-time $metadata fetch. +// +// Only *literal* values are usable here. MDL stores a quoted literal with its +// quotes ('f1api') and a constant reference bare (Module.ApiUser); at design +// time mxcli has no runtime to resolve a constant against, so a reference is +// reported rather than sent as if it were the value itself. +type metadataFetchAuth struct { + Username string // literal, quotes already stripped + Password string // literal, quotes already stripped + Headers map[string]string // literal values only + // Unresolved names the caller referenced by constant, for the message. + Unresolved []string +} + +// apply sets basic auth and headers on the request. A nil receiver is a no-op, +// so an unauthenticated fetch needs no special case at the call site. +func (a *metadataFetchAuth) apply(req *http.Request) { + if a == nil { + return + } + if a.Username != "" || a.Password != "" { + req.SetBasicAuth(a.Username, a.Password) + } + for k, v := range a.Headers { + req.Header.Set(k, v) + } +} + +// metadataAuthFromStmt collects the statement's own credentials and headers for +// the design-time fetch, keeping only the literals. +func metadataAuthFromStmt(stmt *ast.CreateODataClientStmt) *metadataFetchAuth { + auth := &metadataFetchAuth{Headers: map[string]string{}} + switch { + case stmt.HttpUsernameIsLiteral: + auth.Username = stmt.HttpUsername + case stmt.HttpUsername != "": + auth.Unresolved = append(auth.Unresolved, "HttpUsername ("+stmt.HttpUsername+")") + } + switch { + case stmt.HttpPasswordIsLiteral: + auth.Password = stmt.HttpPassword + case stmt.HttpPassword != "": + // Named, not printed: a constant reference is a name, but the value it + // resolves to is a secret and this line goes to the console. + auth.Unresolved = append(auth.Unresolved, "HttpPassword ("+stmt.HttpPassword+")") + } + for _, h := range stmt.Headers { + switch { + case h.ValueIsLiteral: + auth.Headers[h.Key] = h.Value + case h.Value != "": + auth.Unresolved = append(auth.Unresolved, "header "+h.Key+" ("+h.Value+")") + } + } + sort.Strings(auth.Unresolved) + return auth +} + +// hints explains a failed fetch when the reason is credentials mxcli could not +// resolve, and points at the workaround that also happens to be better practice. +func (a *metadataFetchAuth) hints() []string { + var out []string + if a == nil { + return out + } + if len(a.Unresolved) > 0 { + out = append(out, "These are constant references, which only the runtime can resolve, so the fetch went out without them: "+strings.Join(a.Unresolved, ", ")) + } + out = append(out, + "Fetch the contract once and point MetadataUrl at the file — it commits, so the", + "model rebuilds without the service running and a contract change is a reviewable diff.") + return out +} + +func fetchODataMetadata(metadataUrl string, auth *metadataFetchAuth) (metadata string, hash string, err error) { if metadataUrl == "" { return "", "", nil } @@ -1818,7 +1962,16 @@ func fetchODataMetadata(metadataUrl string) (metadata string, hash string, err e } else { // HTTP(S) fetch client := &http.Client{Timeout: 30 * time.Second} - resp, err := client.Get(metadataUrl) + req, reqErr := http.NewRequest(http.MethodGet, metadataUrl, nil) + if reqErr != nil { + return "", "", mdlerrors.NewBackend(fmt.Sprintf("build $metadata request for %s", metadataUrl), reqErr) + } + // The credentials and headers already on the statement apply to this + // fetch too. Without them a service behind `authentication basic` + // answers 401, the client is created with no cached entity types, and + // the CREATE EXTERNAL ENTITIES that follows silently imports nothing. + auth.apply(req) + resp, err := client.Do(req) if err != nil { return "", "", mdlerrors.NewBackend(fmt.Sprintf("fetch $metadata from %s", metadataUrl), err) } diff --git a/mdl/executor/cmd_odata_edm_type_test.go b/mdl/executor/cmd_odata_edm_type_test.go new file mode 100644 index 000000000..1781183f3 --- /dev/null +++ b/mdl/executor/cmd_odata_edm_type_test.go @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// An enumeration published as Edm.String must also carry EnumerationAsString. +// The pair is one setting: with the flag false, Mendix wants the enumeration +// published in the service as its own EDM enum type and rejects the string — +// CE5016 plus CE4583 "Enumeration 'Edm.Colour' is not published in this +// service". mxcli wrote Edm.String with the flag hardcoded false, which is the +// one combination that cannot build. +func TestEnumerationPublishesAsString(t *testing.T) { + if !enumPublishedAsString(&domainmodel.EnumerationAttributeType{}) { + t.Error("an enumeration attribute must set EnumerationAsString") + } + // A plain String also publishes as Edm.String but is not an enumeration — + // the flag is what tells the two apart, so it must not be set here. + if enumPublishedAsString(&domainmodel.StringAttributeType{}) { + t.Error("a String attribute must not set EnumerationAsString") + } + if enumPublishedAsString(nil) { + t.Error("a nil type must not set EnumerationAsString") + } +} diff --git a/mdl/executor/cmd_odata_metadata_auth_test.go b/mdl/executor/cmd_odata_metadata_auth_test.go new file mode 100644 index 000000000..455286d29 --- /dev/null +++ b/mdl/executor/cmd_odata_metadata_auth_test.go @@ -0,0 +1,97 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// mxcli-formula1 findings #23: CREATE ODATA CLIENT accepts UseAuthentication / +// HttpUsername / HttpPassword and stores them for the runtime, but the +// design-time $metadata fetch was a bare client.Get. Against a service behind +// `authentication basic` that is a 401, and because the fetch failure is only a +// warning the client is created with no cached entity types — so the +// CREATE EXTERNAL ENTITIES that follows imports nothing and the script looks +// like it succeeded. +func TestFetchODataMetadata_SendsCredentialsAndHeaders(t *testing.T) { + const body = `` + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + user, pass, ok := r.BasicAuth() + if !ok || user != "f1api" || pass != "s3cret" { + w.WriteHeader(http.StatusUnauthorized) + return + } + if r.Header.Get("X-Probe") != "yes" { + w.WriteHeader(http.StatusForbidden) + return + } + _, _ = w.Write([]byte(body)) + })) + defer srv.Close() + + auth := &metadataFetchAuth{ + Username: "f1api", + Password: "s3cret", + Headers: map[string]string{"X-Probe": "yes"}, + } + got, hash, err := fetchODataMetadata(srv.URL, auth) + if err != nil { + t.Fatalf("fetch with credentials failed: %v", err) + } + if got != body { + t.Errorf("body = %q, want the served metadata", got) + } + if hash == "" { + t.Error("no hash computed for a successful fetch") + } + + // Without them, the same service is a 401 — which is what shipped. + if _, _, err := fetchODataMetadata(srv.URL, nil); err == nil { + t.Error("unauthenticated fetch succeeded, so the test server proves nothing") + } +} + +// A literal is usable at design time; a constant reference is not — the runtime +// resolves those, mxcli has nothing to resolve them against. Sending the +// constant's *name* as the password would be worse than sending nothing, so the +// name is reported instead. +func TestMetadataAuthFromStmt_LiteralsOnly(t *testing.T) { + stmt := &ast.CreateODataClientStmt{ + HttpUsername: "f1api", + HttpUsernameIsLiteral: true, + HttpPassword: "Module.ApiPassword", // a constant reference + Headers: []ast.HeaderDef{ + {Key: "X-Probe", Value: "yes", ValueIsLiteral: true}, + {Key: "X-Token", Value: "Module.Token"}, + }, + } + auth := metadataAuthFromStmt(stmt) + + if auth.Username != "f1api" { + t.Errorf("Username = %q, want the literal f1api", auth.Username) + } + if auth.Password != "" { + t.Errorf("Password = %q, want empty — a constant reference is not a value", auth.Password) + } + if auth.Headers["X-Probe"] != "yes" { + t.Errorf("literal header dropped: %v", auth.Headers) + } + if _, ok := auth.Headers["X-Token"]; ok { + t.Error("a constant-reference header was sent as its own name") + } + // Both unresolved names are reported, sorted, so the user learns why the + // fetch went out unauthenticated. + want := []string{"HttpPassword (Module.ApiPassword)", "header X-Token (Module.Token)"} + if len(auth.Unresolved) != len(want) { + t.Fatalf("Unresolved = %v, want %v", auth.Unresolved, want) + } + for i := range want { + if auth.Unresolved[i] != want[i] { + t.Errorf("Unresolved[%d] = %q, want %q", i, auth.Unresolved[i], want[i]) + } + } +} diff --git a/mdl/executor/cmd_odata_modify_members_test.go b/mdl/executor/cmd_odata_modify_members_test.go new file mode 100644 index 000000000..0e7cf7d55 --- /dev/null +++ b/mdl/executor/cmd_odata_modify_members_test.go @@ -0,0 +1,143 @@ +// 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/domainmodel" +) + +// mxcli-formula1 findings #26: re-running a `create or modify odata service` +// after editing a `publish entity` block did not apply the change. Marking a +// member Filterable and re-executing left the served $metadata exactly as it +// was; only `drop odata service` + create picked it up. The modify branch +// updated the service's scalar properties and never touched EntityTypes or +// EntitySets. +// +// Confirmed on 11.12.1 against a real build: the same script produced +// `Label as 'label'` before the fix and `Label as 'label' (Filterable, Sortable)` +// after. +func TestModifyODataService_AppliesPublishedEntities(t *testing.T) { + svc, mb, h := existingPublishedService() + var updated *model.PublishedODataService + mb.UpdatePublishedODataServiceFunc = func(s *model.PublishedODataService) error { + updated = s + return nil + } + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h)) + + stmt := &ast.CreateODataServiceStmt{ + Name: ast.QualifiedName{Module: "MyModule", Name: "CatalogService"}, + CreateOrModify: true, + Entities: []*ast.PublishedEntityDef{{ + Entity: ast.QualifiedName{Module: "MyModule", Name: "Order"}, + ExposedName: "Orders", + Members: []*ast.PublishedMemberDef{ + {Name: "Label", ExposedName: "label", Filterable: true, Sortable: true}, + }, + }}, + } + assertNoError(t, createODataService(ctx, stmt)) + + if updated == nil { + t.Fatal("the service was never updated") + } + m := findPublishedMember(t, updated, "Label") + if !m.Filterable || !m.Sortable { + t.Errorf("member Label: filterable=%v sortable=%v, want both true — "+ + "the modify did not apply the edited publish block", m.Filterable, m.Sortable) + } + _ = svc +} + +// A modify cannot express role grants (`grant access on odata service …` is a +// separate statement), so it must carry the existing ones through or the build +// fails with "At least one allowed role must be selected". +// +// A guard rather than a reproduction: the loss was reported but did not +// reproduce on 11.12.1 — grants survived a modify on both the fixed and the +// previous build. The invariant holds regardless. +func TestModifyODataService_KeepsRoleGrants(t *testing.T) { + svc, mb, h := existingPublishedService() + svc.AllowedModuleRoles = []string{"MyModule.ApiUser"} + var updated *model.PublishedODataService + mb.UpdatePublishedODataServiceFunc = func(s *model.PublishedODataService) error { + updated = s + return nil + } + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h)) + + stmt := &ast.CreateODataServiceStmt{ + Name: ast.QualifiedName{Module: "MyModule", Name: "CatalogService"}, + CreateOrModify: true, + Path: "odata/catalog2/", + } + assertNoError(t, createODataService(ctx, stmt)) + + if updated == nil { + t.Fatal("the service was never updated") + } + if len(updated.AllowedModuleRoles) != 1 || updated.AllowedModuleRoles[0] != "MyModule.ApiUser" { + t.Errorf("AllowedModuleRoles = %v, want [MyModule.ApiUser]", updated.AllowedModuleRoles) + } + if updated.Path != "odata/catalog2/" { + t.Errorf("Path = %q, want the modify's own change to land too", updated.Path) + } +} + +// existingPublishedService is a one-entity service already in the model, with +// the backend and hierarchy wired so createODataService takes its modify branch. +func existingPublishedService() (*model.PublishedODataService, *mock.MockBackend, *ContainerHierarchy) { + mod := mkModule("MyModule") + svc := &model.PublishedODataService{ + ContainerID: mod.ID, + Name: "CatalogService", + Path: "odata/catalog/", + ServiceName: "CatalogService", + EntityTypes: []*model.PublishedEntityType{{ + ExposedName: "Order", + Entity: "MyModule.Order", + Members: []*model.PublishedMember{{Kind: "attribute", Name: "Label", ExposedName: "label"}}, + }}, + EntitySets: []*model.PublishedEntitySet{{ + ExposedName: "Orders", + EntityTypeName: "MyModule.Order", + }}, + } + h := mkHierarchy(mod) + withContainer(h, svc.ContainerID, mod.ID) + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + GetModuleByNameFunc: func(string) (*model.Module, error) { return mod, nil }, + GetDomainModelFunc: func(model.ID) (*domainmodel.DomainModel, error) { + return &domainmodel.DomainModel{ + Entities: []*domainmodel.Entity{{ + Name: "Order", + Attributes: []*domainmodel.Attribute{{Name: "Label", Type: &domainmodel.StringAttributeType{Length: 120}}}, + }}, + }, nil + }, + ListPublishedODataServicesFunc: func() ([]*model.PublishedODataService, error) { + return []*model.PublishedODataService{svc}, nil + }, + } + return svc, mb, h +} + +func findPublishedMember(t *testing.T, svc *model.PublishedODataService, name string) *model.PublishedMember { + t.Helper() + for _, et := range svc.EntityTypes { + for _, m := range et.Members { + if m.Name == name { + return m + } + } + } + t.Fatalf("published member %q not found", name) + return nil +} diff --git a/mdl/executor/cmd_odata_test.go b/mdl/executor/cmd_odata_test.go index 9df6b8f00..166dde9e8 100644 --- a/mdl/executor/cmd_odata_test.go +++ b/mdl/executor/cmd_odata_test.go @@ -47,7 +47,7 @@ func TestFetchODataMetadata_LocalFile(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - metadata, hash, err := fetchODataMetadata(tt.url) + metadata, hash, err := fetchODataMetadata(tt.url, nil) if tt.wantErr { if err == nil { @@ -72,7 +72,7 @@ func TestFetchODataMetadata_LocalFile(t *testing.T) { } // Hash should be consistent - _, hash2, _ := fetchODataMetadata(tt.url) + _, hash2, _ := fetchODataMetadata(tt.url, nil) if hash != hash2 { t.Errorf("Hash inconsistent between calls: %q vs %q", hash, hash2) } @@ -182,7 +182,7 @@ func TestFetchODataMetadata_LocalFileAbsolute(t *testing.T) { fileURL = "file:///" + filepath.ToSlash(filePath) } - metadata, hash, err := fetchODataMetadata(fileURL) + metadata, hash, err := fetchODataMetadata(fileURL, nil) if err != nil { t.Errorf("Unexpected error: %v", err) } diff --git a/mdl/executor/cmd_security_write.go b/mdl/executor/cmd_security_write.go index 3eb8fdbed..52aa3bb4d 100644 --- a/mdl/executor/cmd_security_write.go +++ b/mdl/executor/cmd_security_write.go @@ -60,6 +60,18 @@ func execCreateModuleRole(ctx *ExecContext, s *ast.CreateModuleRoleStmt) error { } return nil } + if s.CreateOrModify { + // Re-running a security script must not fail on a role that is + // already there. AddModuleRole overwrites, so this also adopts a new + // description and the caller's casing. + if err := ctx.Backend.AddModuleRole(ms.ID, s.Name.Name, s.Description); err != nil { + return mdlerrors.NewBackend("modify module role", err) + } + if !ctx.Quiet { + fmt.Fprintf(ctx.Output, "Modified module role: %s.%s\n", s.Name.Module, s.Name.Name) + } + return nil + } return mdlerrors.NewAlreadyExists("module role", s.Name.Module+"."+s.Name.Name) } diff --git a/mdl/executor/validate_odata_properties.go b/mdl/executor/validate_odata_properties.go index eb666627a..9119ce961 100644 --- a/mdl/executor/validate_odata_properties.go +++ b/mdl/executor/validate_odata_properties.go @@ -21,6 +21,11 @@ import ( // Known property names, in the spelling the syntax help uses. These are for the // error message only — the visitor is the authority on what is accepted, and it // matches case-insensitively. +// +// The two drifted once already: Countable/SkipSupported/TopSupported were added +// to the visitor and the hint went on advertising six properties, so a user +// reading it would think three accepted properties were not. TestKnownODataProps +// keeps them in step by running every name below through the visitor. var ( knownODataServiceProps = []string{ "Path", "Version", "ODataVersion", "Namespace", "ServiceName", @@ -28,6 +33,7 @@ var ( } knownPublishEntityProps = []string{ "ReadMode", "InsertMode", "UpdateMode", "DeleteMode", "UsePaging", "PageSize", + "Countable", "SkipSupported", "TopSupported", } knownODataClientProps = []string{ "Version", "ODataVersion", "MetadataUrl", "Timeout", "ProxyType", diff --git a/mdl/executor/validate_odata_properties_drift_test.go b/mdl/executor/validate_odata_properties_drift_test.go new file mode 100644 index 000000000..08cef446a --- /dev/null +++ b/mdl/executor/validate_odata_properties_drift_test.go @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "reflect" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// MDL-ODATA01's hint listed six publish-entity properties long after the visitor +// grew three more (Countable / SkipSupported / TopSupported), so the message told +// users that accepted properties were unknown. The lists are separate by design — +// the visitor decides, the hint only displays — but they must not drift, and +// nothing was checking (mxcli-formula1 #15). +// +// The visitor is the authority: every name the hint advertises is fed through it +// and must not come back as unknown. +func TestKnownODataProps_MatchTheVisitor(t *testing.T) { + cases := []struct { + what string + props []string + build func(prop string) string + }{ + {"odata service", knownODataServiceProps, func(p string) string { + return fmt.Sprintf("create odata service M.S (%s: 'x');", p) + }}, + {"publish entity", knownPublishEntityProps, func(p string) string { + return fmt.Sprintf("create odata service M.S (Path: 'p/')\n{\n publish entity M.E as 'Es' (%s: 'x')\n expose (A);\n};", p) + }}, + {"odata client", knownODataClientProps, func(p string) string { + return fmt.Sprintf("create odata client M.C (%s: 'x');", p) + }}, + {"external entity", knownExternalEntityProps, func(p string) string { + return fmt.Sprintf("create external entity M.E from odata client M.C (%s: 'x');", p) + }}, + } + + for _, tc := range cases { + for _, prop := range tc.props { + t.Run(tc.what+"/"+prop, func(t *testing.T) { + prog, errs := visitor.Build(tc.build(prop)) + if len(errs) > 0 { + t.Fatalf("%q does not parse in a %s: %v", prop, tc.what, errs) + } + if unknown := collectUnknownProps(prog); len(unknown) > 0 { + t.Errorf("the hint advertises %q but the visitor discards it (unknown: %v)", prop, unknown) + } + }) + } + } +} + +// The direction that actually broke: a property the AST carries but the hint does +// not advertise. Countable/SkipSupported/TopSupported were added as fields, the +// visitor learned to set them, and the hint was never updated — so the message +// told users three accepted properties were unknown. +// +// The AST struct is the single source here: every field is a property unless it +// is listed as structural, so adding one and forgetting the hint fails this test +// rather than shipping a wrong message. +func TestKnownODataProps_CoverEveryASTField(t *testing.T) { + cases := []struct { + what string + typ reflect.Type + advertised []string + structural map[string]bool + }{ + { + "publish entity", reflect.TypeOf(ast.PublishedEntityDef{}), knownPublishEntityProps, + map[string]bool{"Entity": true, "ExposedName": true, "Members": true, "UnknownProperties": true}, + }, + { + "external entity", reflect.TypeOf(ast.CreateExternalEntityStmt{}), knownExternalEntityProps, + map[string]bool{ + "Name": true, "ServiceRef": true, "Attributes": true, "Documentation": true, + "CreateOrModify": true, "UnknownProperties": true, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.what, func(t *testing.T) { + have := map[string]bool{} + for _, p := range tc.advertised { + have[strings.ToLower(p)] = true + } + for i := 0; i < tc.typ.NumField(); i++ { + f := tc.typ.Field(i) + if tc.structural[f.Name] || !f.IsExported() { + continue + } + // Flags that only record whether a sibling was set are not + // properties in their own right. + if strings.HasSuffix(f.Name, "IsLiteral") || strings.HasSuffix(f.Name, "Set") || + strings.HasSuffix(f.Name, "IsExpression") { + continue + } + if !have[strings.ToLower(f.Name)] { + t.Errorf("%s carries %s but MDL-ODATA01 does not advertise it — "+ + "a user typing that property is told it is unknown", tc.what, f.Name) + } + } + }) + } +} + +// The converse: a name nothing accepts must still be reported, or the test above +// would pass against a visitor that silently swallowed everything. +func TestKnownODataProps_UnknownIsStillFlagged(t *testing.T) { + prog, errs := visitor.Build("create odata client M.C (NotAProperty: 'x');") + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + unknown := collectUnknownProps(prog) + if len(unknown) != 1 || !strings.EqualFold(unknown[0], "NotAProperty") { + t.Errorf("unknown = %v, want [NotAProperty]", unknown) + } +} + +func collectUnknownProps(prog *ast.Program) []string { + var out []string + for _, stmt := range prog.Statements { + switch s := stmt.(type) { + case *ast.CreateODataServiceStmt: + out = append(out, s.UnknownProperties...) + for _, e := range s.Entities { + if e != nil { + out = append(out, e.UnknownProperties...) + } + } + case *ast.CreateODataClientStmt: + out = append(out, s.UnknownProperties...) + case *ast.CreateExternalEntityStmt: + out = append(out, s.UnknownProperties...) + } + } + return out +} diff --git a/mdl/grammar/MDLParser.g4 b/mdl/grammar/MDLParser.g4 index 7b2d2e0a9..ad7b02d17 100644 --- a/mdl/grammar/MDLParser.g4 +++ b/mdl/grammar/MDLParser.g4 @@ -363,8 +363,8 @@ renameTarget * ``` */ moveStatement - : MOVE (PAGE | MICROFLOW | SNIPPET | NANOFLOW | ENUMERATION | CONSTANT | DATABASE CONNECTION) qualifiedName TO FOLDER STRING_LITERAL (IN (qualifiedName | IDENTIFIER))? - | MOVE (PAGE | MICROFLOW | SNIPPET | NANOFLOW | ENUMERATION | CONSTANT | DATABASE CONNECTION) qualifiedName TO (qualifiedName | IDENTIFIER) + : MOVE (PAGE | MICROFLOW | SNIPPET | NANOFLOW | ENUMERATION | CONSTANT | DATABASE CONNECTION | JAVA ACTION | ODATA SERVICE) qualifiedName TO FOLDER STRING_LITERAL (IN (qualifiedName | IDENTIFIER))? + | MOVE (PAGE | MICROFLOW | SNIPPET | NANOFLOW | ENUMERATION | CONSTANT | DATABASE CONNECTION | JAVA ACTION | ODATA SERVICE) qualifiedName TO (qualifiedName | IDENTIFIER) | MOVE ENTITY qualifiedName TO (qualifiedName | IDENTIFIER) | MOVE FOLDER qualifiedName TO FOLDER STRING_LITERAL (IN (qualifiedName | IDENTIFIER))? | MOVE FOLDER qualifiedName TO (qualifiedName | IDENTIFIER) diff --git a/mdl/grammar/domains/MDLSecurity.g4 b/mdl/grammar/domains/MDLSecurity.g4 index bcd1c1e45..a49965547 100644 --- a/mdl/grammar/domains/MDLSecurity.g4 +++ b/mdl/grammar/domains/MDLSecurity.g4 @@ -10,8 +10,11 @@ options { tokenVocab = MDLLexer; } // SECURITY STATEMENTS // ============================================================================= +// OR MODIFY makes a security script re-runnable. Without it, re-executing the +// script that sets up roles fails on the first role that already exists, so +// role creation had to live in its own run-once file. createModuleRoleStatement - : CREATE MODULE ROLE qualifiedName (DESCRIPTION STRING_LITERAL)? + : CREATE (OR MODIFY)? MODULE ROLE qualifiedName (DESCRIPTION STRING_LITERAL)? ; dropModuleRoleStatement diff --git a/mdl/types/edmx.go b/mdl/types/edmx.go index 15ac367d1..c04ed35e6 100644 --- a/mdl/types/edmx.go +++ b/mdl/types/edmx.go @@ -77,6 +77,14 @@ type EdmEntitySet struct { Insertable *bool // InsertRestrictions/Insertable Updatable *bool // UpdateRestrictions/Updatable Deletable *bool // DeleteRestrictions/Deletable + Countable *bool // CountRestrictions/Countable + + // Property names the service says cannot be filtered or sorted on, from + // FilterRestrictions/NonFilterableProperties and + // SortRestrictions/NonSortableProperties. Mendix compares these against the + // app's per-attribute flags and reports CE6630 on a mismatch. + NonFilterableProperties []string + NonSortableProperties []string // Navigation property names listed under // Org.OData.Capabilities.V1.{Insert,Update}Restrictions/Non*NavigationProperties. @@ -408,6 +416,25 @@ func applyCapabilityAnnotations(es *EdmEntitySet, annotations []xmlCapabilitiesA es.Deletable = &v } } + case "Org.OData.Capabilities.V1.CountRestrictions": + for _, pv := range ann.Record.PropertyValues { + if pv.Property == "Countable" && pv.Bool != "" { + v := pv.Bool == "true" + es.Countable = &v + } + } + case "Org.OData.Capabilities.V1.FilterRestrictions": + for _, pv := range ann.Record.PropertyValues { + if pv.Property == "NonFilterableProperties" && pv.Collection != nil { + es.NonFilterableProperties = pv.Collection.PropertyPaths + } + } + case "Org.OData.Capabilities.V1.SortRestrictions": + for _, pv := range ann.Record.PropertyValues { + if pv.Property == "NonSortableProperties" && pv.Collection != nil { + es.NonSortableProperties = pv.Collection.PropertyPaths + } + } } } } diff --git a/mdl/types/edmx_test.go b/mdl/types/edmx_test.go index 4814d7209..a0e63178b 100644 --- a/mdl/types/edmx_test.go +++ b/mdl/types/edmx_test.go @@ -398,3 +398,98 @@ func TestParseEdmx_ConcurrencyModeFixed(t *testing.T) { t.Error("ConcurrencyMode='Fixed' must set Computed=true so the attribute is not marked Creatable (issue #525)") } } + +// mxcli-formula1 findings #24: CREATE EXTERNAL ENTITIES read names, types and +// navigation properties out of the contract correctly, then defaulted every +// capability to true regardless of what the contract said. Mendix compares the +// two at build time and refuses: +// +// 'Seasons' is marked Countable=False in the OData service, but True in the app. +// 'latitude' is marked Filterable=False in the OData service, but True in the app. +// +// Insert/Update/Delete restrictions were already parsed; Count/Filter/Sort were +// not, so there was nothing for the import to honour. +func TestParseEdmx_CountFilterSortRestrictions(t *testing.T) { + const md = ` + + + + + + + + + + + + + + + + + latitude + + + + + altitude + + + + + + +` + + doc, err := ParseEdmx(md) + if err != nil { + t.Fatalf("ParseEdmx: %v", err) + } + if len(doc.EntitySets) != 1 { + t.Fatalf("got %d entity sets, want 1", len(doc.EntitySets)) + } + es := doc.EntitySets[0] + + if es.Countable == nil || *es.Countable { + t.Errorf("Countable = %v, want an explicit false", es.Countable) + } + if len(es.NonFilterableProperties) != 1 || es.NonFilterableProperties[0] != "latitude" { + t.Errorf("NonFilterableProperties = %v, want [latitude]", es.NonFilterableProperties) + } + if len(es.NonSortableProperties) != 1 || es.NonSortableProperties[0] != "altitude" { + t.Errorf("NonSortableProperties = %v, want [altitude]", es.NonSortableProperties) + } +} + +// A contract that says nothing must leave the capabilities unspecified, so the +// import keeps OData's own default (countable, filterable, sortable) rather than +// reading silence as a restriction. +func TestParseEdmx_NoRestrictionsLeavesCapabilitiesUnset(t *testing.T) { + const md = ` + + + + + + + + + + + + +` + + doc, err := ParseEdmx(md) + if err != nil { + t.Fatalf("ParseEdmx: %v", err) + } + es := doc.EntitySets[0] + if es.Countable != nil { + t.Errorf("Countable = %v, want nil (unspecified)", *es.Countable) + } + if len(es.NonFilterableProperties) != 0 || len(es.NonSortableProperties) != 0 { + t.Errorf("restrictions invented from an unannotated set: filter=%v sort=%v", + es.NonFilterableProperties, es.NonSortableProperties) + } +} diff --git a/mdl/visitor/visitor_entity.go b/mdl/visitor/visitor_entity.go index 483f5b0d5..c4b043d6e 100644 --- a/mdl/visitor/visitor_entity.go +++ b/mdl/visitor/visitor_entity.go @@ -939,7 +939,8 @@ func (b *Builder) ExitMoveStatement(ctx *parser.MoveStatementContext) { // MOVE FOLDER is identified by having FOLDER as the first token after MOVE (no document type keyword) if len(ctx.AllFOLDER()) > 0 && ctx.PAGE() == nil && ctx.MICROFLOW() == nil && ctx.SNIPPET() == nil && ctx.NANOFLOW() == nil && ctx.ENTITY() == nil && - ctx.ENUMERATION() == nil && ctx.CONSTANT() == nil && ctx.DATABASE() == nil { + ctx.ENUMERATION() == nil && ctx.CONSTANT() == nil && ctx.DATABASE() == nil && + ctx.JAVA() == nil && ctx.ODATA() == nil { b.exitMoveFolderStatement(ctx, names) return } @@ -965,6 +966,10 @@ func (b *Builder) ExitMoveStatement(ctx *parser.MoveStatementContext) { stmt.DocumentType = ast.DocumentTypeConstant } else if ctx.DATABASE() != nil { stmt.DocumentType = ast.DocumentTypeDatabaseConnection + } else if ctx.JAVA() != nil { + stmt.DocumentType = ast.DocumentTypeJavaAction + } else if ctx.ODATA() != nil { + stmt.DocumentType = ast.DocumentTypeODataService } // Parse folder path if specified diff --git a/mdl/visitor/visitor_microflow_actions.go b/mdl/visitor/visitor_microflow_actions.go index fa47a2217..fba6bedff 100644 --- a/mdl/visitor/visitor_microflow_actions.go +++ b/mdl/visitor/visitor_microflow_actions.go @@ -574,6 +574,7 @@ func buildExecuteDatabaseQueryStatement(ctx parser.IExecuteDatabaseQueryStatemen stmt.DynamicQuery = unquoteDollarString(ds.GetText()) } else if expr := execCtx.Expression(); expr != nil { stmt.DynamicQuery = expr.GetText() + stmt.DynamicQueryIsExpression = true } } diff --git a/mdl/visitor/visitor_move_doctypes_test.go b/mdl/visitor/visitor_move_doctypes_test.go new file mode 100644 index 000000000..67fbac839 --- /dev/null +++ b/mdl/visitor/visitor_move_doctypes_test.go @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// mxcli-formula1 findings #32: MOVE accepted seven doctypes and rejected the +// rest at parse time (`no viable alternative at input 'MOVEJAVA'`). Neither +// CREATE JAVA ACTION nor CREATE ODATA SERVICE takes a folder clause either, so +// those documents could never leave the module root from MDL — five of that +// backend's documents were stuck there. +func TestMoveStatement_JavaActionAndODataService(t *testing.T) { + cases := []struct { + src string + wantType ast.DocumentType + wantName string + wantFolder string + }{ + {"move java action Mv.Helper to folder 'Support';", ast.DocumentTypeJavaAction, "Helper", "Support"}, + {"move odata service Mv.Api to folder 'Api/Published';", ast.DocumentTypeODataService, "Api", "Api/Published"}, + // The doctypes that already worked must keep working. + {"move microflow Mv.Flow to folder 'Live';", ast.DocumentTypeMicroflow, "Flow", "Live"}, + {"move database connection Mv.Db to folder 'Warehouse';", ast.DocumentTypeDatabaseConnection, "Db", "Warehouse"}, + } + + for _, tc := range cases { + t.Run(tc.src, func(t *testing.T) { + prog, errs := Build(tc.src) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + var got *ast.MoveStmt + for _, s := range prog.Statements { + if m, ok := s.(*ast.MoveStmt); ok { + got = m + } + } + if got == nil { + t.Fatal("no MoveStmt produced") + } + if got.DocumentType != tc.wantType { + t.Errorf("DocumentType = %q, want %q", got.DocumentType, tc.wantType) + } + if got.Name.Name != tc.wantName { + t.Errorf("Name = %q, want %q", got.Name.Name, tc.wantName) + } + if got.Folder != tc.wantFolder { + t.Errorf("Folder = %q, want %q", got.Folder, tc.wantFolder) + } + }) + } +} + +// MOVE FOLDER is told apart from a document move by the absence of a doctype +// keyword, so adding two more keywords must not make a folder move look like a +// document move. +func TestMoveStatement_FolderStillDistinct(t *testing.T) { + prog, errs := Build("move folder Mv.Old to folder 'New';") + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + for _, s := range prog.Statements { + if m, ok := s.(*ast.MoveStmt); ok { + t.Fatalf("MOVE FOLDER produced a document MoveStmt (%q)", m.DocumentType) + } + } +} diff --git a/mdl/visitor/visitor_odata.go b/mdl/visitor/visitor_odata.go index b45380676..519dac706 100644 --- a/mdl/visitor/visitor_odata.go +++ b/mdl/visitor/visitor_odata.go @@ -45,8 +45,10 @@ func (b *Builder) ExitCreateODataClientStatement(ctx *parser.CreateODataClientSt stmt.UseAuthentication = strings.EqualFold(value, "true") || strings.EqualFold(value, "yes") case "httpusername": stmt.HttpUsername = value + stmt.HttpUsernameIsLiteral = odataValueIsLiteral(prop) case "httppassword": stmt.HttpPassword = value + stmt.HttpPasswordIsLiteral = odataValueIsLiteral(prop) case "clientcertificate": stmt.ClientCertificate = value case "configurationmicroflow": @@ -296,6 +298,18 @@ func odataValueText(val *parser.OdataPropertyValueContext) string { return "" } +// odataValueIsLiteral reports whether an OData property value was written as a +// quoted string rather than a constant reference. odataValueText strips a +// literal's quotes, so this is the only thing that still tells the two apart — +// and mxcli can only use a literal for the design-time $metadata fetch. +func odataValueIsLiteral(prop *parser.OdataPropertyAssignmentContext) bool { + valCtx := prop.OdataPropertyValue() + if valCtx == nil { + return false + } + return valCtx.(*parser.OdataPropertyValueContext).STRING_LITERAL() != nil +} + // odataAssignmentValueText extracts the string value from an OData property assignment. func odataAssignmentValueText(prop *parser.OdataPropertyAssignmentContext) string { valCtx := prop.OdataPropertyValue() @@ -390,10 +404,13 @@ func parseODataHeaders(ctx parser.IOdataHeadersClauseContext) []ast.HeaderDef { entry := entryCtx.(*parser.OdataHeaderEntryContext) key := unquoteString(entry.STRING_LITERAL().GetText()) value := "" + isLiteral := false if valCtx := entry.OdataPropertyValue(); valCtx != nil { - value = odataValueText(valCtx.(*parser.OdataPropertyValueContext)) + vc := valCtx.(*parser.OdataPropertyValueContext) + value = odataValueText(vc) + isLiteral = vc.STRING_LITERAL() != nil } - headers = append(headers, ast.HeaderDef{Key: key, Value: value}) + headers = append(headers, ast.HeaderDef{Key: key, Value: value, ValueIsLiteral: isLiteral}) } return headers diff --git a/mdl/visitor/visitor_security.go b/mdl/visitor/visitor_security.go index fd91ff405..b09cb7213 100644 --- a/mdl/visitor/visitor_security.go +++ b/mdl/visitor/visitor_security.go @@ -14,7 +14,8 @@ func (b *Builder) ExitCreateModuleRoleStatement(ctx *parser.CreateModuleRoleStat return } stmt := &ast.CreateModuleRoleStmt{ - Name: buildQualifiedName(qn), + Name: buildQualifiedName(qn), + CreateOrModify: ctx.MODIFY() != nil, } if ctx.DESCRIPTION() != nil { if sl := ctx.STRING_LITERAL(); sl != nil { diff --git a/mdl/visitor/visitor_security_or_modify_test.go b/mdl/visitor/visitor_security_or_modify_test.go new file mode 100644 index 000000000..22521e404 --- /dev/null +++ b/mdl/visitor/visitor_security_or_modify_test.go @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// mxcli-formula1 suggested issue 12: `create module role` had no `or modify` +// form, so a security script failed on the first role that already existed and +// role creation had to live in its own run-once file. +func TestCreateModuleRole_OrModify(t *testing.T) { + cases := []struct { + src string + wantOrModify bool + wantName, desc string + }{ + {"create module role Sec.ApiUser;", false, "ApiUser", ""}, + {"create or modify module role Sec.ApiUser;", true, "ApiUser", ""}, + {"create or modify module role Sec.ApiUser description 'API consumer';", true, "ApiUser", "API consumer"}, + } + for _, tc := range cases { + t.Run(tc.src, func(t *testing.T) { + prog, errs := Build(tc.src) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + var got *ast.CreateModuleRoleStmt + for _, s := range prog.Statements { + if r, ok := s.(*ast.CreateModuleRoleStmt); ok { + got = r + } + } + if got == nil { + t.Fatal("no CreateModuleRoleStmt produced") + } + if got.CreateOrModify != tc.wantOrModify { + t.Errorf("CreateOrModify = %v, want %v", got.CreateOrModify, tc.wantOrModify) + } + if got.Name.Name != tc.wantName { + t.Errorf("Name = %q, want %q", got.Name.Name, tc.wantName) + } + if got.Description != tc.desc { + t.Errorf("Description = %q, want %q", got.Description, tc.desc) + } + }) + } +} + +// `create module` and `create module role` differ only after the third token, so +// the optional OR MODIFY must not make one shadow the other. +func TestCreateModuleAndModuleRoleStayDistinct(t *testing.T) { + prog, errs := Build("create or modify module Sec;\ncreate or modify module role Sec.ApiUser;") + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + var modules, roles int + for _, s := range prog.Statements { + switch s.(type) { + case *ast.CreateModuleStmt: + modules++ + case *ast.CreateModuleRoleStmt: + roles++ + } + } + if modules != 1 || roles != 1 { + t.Errorf("got %d module and %d module-role statements, want 1 each", modules, roles) + } +} diff --git a/model/types.go b/model/types.go index 7ee133361..6435c5127 100644 --- a/model/types.go +++ b/model/types.go @@ -496,6 +496,13 @@ type PublishedMember struct { // without it `mx check` reports CE5016 ("published as ."). Attribute members only. EdmType string `json:"edmType,omitempty"` + // EnumerationAsString publishes an enumeration attribute as Edm.String + // rather than as its own EDM enum type. The two are one setting, not two: + // with this false, Mendix expects the enumeration itself to be published in + // the service and rejects Edm.String (CE5016 + CE4583 "Enumeration is not + // published in this service"). Attribute members only. + EnumerationAsString bool `json:"enumerationAsString,omitempty"` + // Association-specific fields (Kind == "association"). Studio Pro's // ODataPublish$PublishedAssociationEnd records both the association // target entity (qualified name) and the bare association name diff --git a/sdk/mpr/writer_domainmodel.go b/sdk/mpr/writer_domainmodel.go index fe755c0bc..4fac522f7 100644 --- a/sdk/mpr/writer_domainmodel.go +++ b/sdk/mpr/writer_domainmodel.go @@ -334,6 +334,13 @@ func (w *Writer) UpdateOqlQueriesForMovedEntity(oldQualifiedName, newQualifiedNa } // moveUnitByID changes a unit's ContainerID without modifying its contents. +// MoveUnitByID reparents any top-level document unit. Exported so backends can +// move doctypes that have no dedicated writer method of their own (Java actions, +// published OData services) — the containment row is all that changes. +func (w *Writer) MoveUnitByID(unitID string, newContainerID string) error { + return w.moveUnitByID(unitID, newContainerID) +} + func (w *Writer) moveUnitByID(unitID string, newContainerID string) error { unitIDBlob := uuidToBlob(unitID) containerIDBlob := uuidToBlob(newContainerID) diff --git a/sdk/mpr/writer_odata.go b/sdk/mpr/writer_odata.go index 4b8d3ee60..e79557311 100644 --- a/sdk/mpr/writer_odata.go +++ b/sdk/mpr/writer_odata.go @@ -411,7 +411,7 @@ func serializePublishedMember(m *model.PublishedMember, ownerQN string) bson.D { doc = append(doc, bson.E{Key: "Filterable", Value: m.Filterable}) doc = append(doc, bson.E{Key: "Sortable", Value: m.Sortable}) doc = append(doc, bson.E{Key: "IsPartOfKey", Value: m.IsPartOfKey}) - doc = append(doc, bson.E{Key: "EnumerationAsString", Value: false}) + doc = append(doc, bson.E{Key: "EnumerationAsString", Value: m.EnumerationAsString}) doc = append(doc, bson.E{Key: "StringAsGuid", Value: false}) case "association": doc = append(doc, bson.E{Key: "$Type", Value: "ODataPublish$PublishedAssociationEnd"})