diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index f4b24d120..0536a7267 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -316,6 +316,10 @@ cases for these three BSON types — they fell to `default: return nil`. | `mdl/executor/cmd_microflows_format_action.go` | Added 3 formatter cases | | `mdl/executor/cmd_microflows_format_listop_test.go` | Added 4 formatter tests | | `sdk/mpr/parser_listoperation_test.go` | New file, 4 parser tests | +| `UPDATE SECURITY`, `CREATE ASSOCIATION` or any `GRANT` silently strips **inherited** members from a specialized entity's access rules; `GRANT` naming an inherited member reports success and persists nothing, so REVOKE+GRANT cannot repair it. `mx check` shows only CE0066, hiding the CE2729 "No read access to attribute" cascade until Studio Pro's Update security is clicked | Mendix inheritance is multi-table: all of a parent's attributes are members of the child, so an access rule needs a MemberAccess entry for every member, own **and** inherited, each qualified against the entity that **declares** it. Both the GRANT builder and `ReconcileMemberAccesses` enumerated only `entity.Attributes`, so an inherited reference matched nothing and was deleted as stale — and reconciliation runs **immediately after every GRANT**, deleting what the grant had just written correctly | `mdl/executor/entity_hierarchy.go` (`EntityMembers`), `mdl/executor/cmd_security_write.go` (`execGrantEntityAccess`), `mdl/backend/modelsdk/domainmodel_security_write.go` (`ReconcileMemberAccesses`, `attrRefBelongsTo`), `sdk/mpr/writer_security.go` (legacy engine) | Walk the generalization chain and qualify each member against its declaring entity; in the reconciler, only strip a reference qualified to **this** entity — an ancestor may live in another module or System, neither loaded there, so preserve what cannot be validated. **Two facts must be established against `mx check`, never inferred**: (a) the child-qualified form is CE1613 "attribute no longer exists" while the declaring-entity form validates clean; (b) `System.User`'s members are the exception — entities specialising it are *user entities* whose platform members Mendix manages, and listing them turns a clean rule into CE0066, while omitting `System.FileDocument`'s six members is CE0066 until all are present. **Generalisable**: when a post-write reconcile pass validates against a narrower model than the writer used, it will quietly undo correct writes — check what runs *after* a write before concluding the writer is at fault. Repro `mdl-examples/bug-tests/758-inherited-member-access.mdl`. Issues #758, #765 (umbrella; #451 is the same declaring-entity rule in the change-object writer) | +| `describe` (and `context` / `diff-local`) renders a Retrieve's XPath with only its **first** predicate group — `where A/B[EndDate = $X];` when the BSON holds `[A/B[EndDate = $X]][Status != 'Completed'][CompletionDate = empty]`. No warning; the output reads as a complete but materially *less restrictive* query, so correct defensive code looks buggy | The grammar's `xpathConstraint` rule matches ONE bracket group, and Mendix concatenates siblings. `ParseXPathConstraint` removes the error listeners, so ANTLR parsed group 1, left the rest on the token stream, and **still returned ok=true**; `enrichXPathConstraintForDescribe` treated that as a full parse and re-rendered only what came back. The `if !ok { return original }` fallback never fired | `mdl/visitor/visitor_xpath_public.go` (`ParseXPathConstraint`), `mdl/visitor/xpath_groups.go` (`SplitXPathPredicateGroups`), `mdl/executor/cmd_microflows_format_action.go` (`enrichXPathGroups`, and the render-path split) | Two layers. (1) Reject a partial parse — after the rule, require `stream.LA(1) == antlr.TokenEOF`; that alone stops the loss, since the caller then falls back to the stored string. (2) Split into top-level groups and enrich each, so enrichment still reaches groups after the first. The splitter must track **nesting depth and quoting**: a naive `][` split mangles a nested `[A/B[x = 1]]` and a literal containing `]`. **Generalisable**: a parser that silently accepts a prefix is worse than one that fails — any `ok` returned by a rule that can match less than its input must be checked against EOF before callers treat it as lossless. Repro `mdl-examples/bug-tests/772-xpath-constraint-groups.mdl`; A/B against a pre-fix binary on the same project shows the two dropped groups. Issue #772 | +| An import/export mapping over an entity created with `EXTENDS` maps only its **own** attributes; every inherited field shows unmapped in Studio Pro, and `mx check` reports CE1613 "The selected attribute 'Mod.Child.Attr' no longer exists". An inherited Boolean/DateTime element also gets `DataType=String` | The mapping builder prefixed the entity being mapped unconditionally (`attr = parentEntity + "." + attr`), but a member reference is qualified against the entity that **declares** it — the same rule as entity access rules (#758) and the change-object writer (#451). Separately `resolveAttributeType` scanned only the entity's own attributes and fell through to its `"String"` default | `mdl/executor/cmd_import_mappings.go` and `cmd_export_mappings.go` (both carry the same two lines), `mdl/executor/entity_hierarchy.go` (`ResolveMemberRef`, `ResolveMemberType`) | Route both sites through the generalization walk added for #758: `ResolveMemberRef` returns the declaring-entity reference and `ResolveMemberType` finds the type up the chain, each falling back to the old behaviour when the member cannot be resolved. **Watch for the sibling defect**: the old `resolveAttributeType` matched entities **by name across every domain model**, so a same-named entity in another module could win — resolve the module by name instead. **Generalisable**: when one rule has several call sites, a fix at one of them proves nothing about the others; grep for the *pattern* (`range entity.Attributes`, `parentEntity + "."`) rather than the reported symptom. Repro `mdl-examples/bug-tests/703-mapping-inherited-attributes.mdl`; A/B on the same project shows `Map703.Contract.DocName` (CE1613) become `Map703.DocumentBase.DocName`. Issue #703, umbrella #765 | +| `alter settings model JavaVersion = 'Java21'` on Mendix 11.12+ produces a project mxbuild refuses to **load**: `mx check` reports `System.ArgumentOutOfRangeException ... (Parameter 'majorVersion is an unsupported value: Java21')` at `JavaVersionExtensions.fromString`. Every check downstream of the settings unit is lost with it | Mendix renamed the property between 11.6 (`JavaVersion` = `"Java21"`) and 11.12 (`JavaMajorVersion` = `"21"`) — and the rename changed the **value format** as well as the key. The #759 fix followed only the key, writing the caller's value through verbatim, so the 11.6 spelling landed in the 11.12 key | `mdl/settingsoverlay/settingsoverlay.go` (`JavaVersionValue`, `SetJavaVersion`) — shared by both engines; the dead third copy in `modelsdk/mpr/serialize_services.go` carried it too | Render the value in the dialect the stored key expects: strip/add the `Java` prefix per key, and pass an unrecognisable value through untouched so a typo surfaces as a Mendix error instead of a mangled setting. **Generalisable**: a renamed property is not only a renamed key — check whether the value encoding moved with it, and cover *both* directions (either spelling in, document's dialect out). Note the sharper failure mode: the original #759 shape was an unknown property, which mxbuild **tolerates**, so only Studio Pro broke; a wrong *value* for a known enum is a hard build failure, which is why this one surfaced as a red nightly rather than a user report. Repro `mdl-examples/bug-tests/759-java-version-value-dialect.mdl`. Issue #759 (follow-up) | **Key insight:** `microflows$ListRange` stores offset/limit inside a nested `CustomRange` map — must cast `raw["CustomRange"].(map[string]any)` before diff --git a/.claude/skills/mendix/generate-domain-model.md b/.claude/skills/mendix/generate-domain-model.md index 1e62b7acd..633d04961 100644 --- a/.claude/skills/mendix/generate-domain-model.md +++ b/.claude/skills/mendix/generate-domain-model.md @@ -210,6 +210,15 @@ create persistent entity Module.Photo ( **Note:** `mxcli syntax entity` output may show EXTENDS after `)` — this is misleading. Always place EXTENDS before `(`. +**Security follows inheritance.** Mendix inheritance is multi-table: all of the +parent's attributes are members of the child, so a specialized entity's access rule +must cover them. Grant an inherited member exactly like one of the entity's own — +`grant Module.Viewer on Module.Attachment (read (AttachmentDescription, "Name", Size));` +— and `read *` / `write *` cover them too. Skipping them is Mendix CE0066 "Entity +access is out of date". The one exception is entities extending `System.User`, whose +inherited platform members Mendix manages and which must not be granted. See +`manage-security.md`. + #### System Attributes (Auditing) Mendix supports four built-in auditing properties on persistent entities. Declare them as regular attributes using pseudo-types (like `autonumber`): diff --git a/.claude/skills/mendix/json-structures-and-mappings.md b/.claude/skills/mendix/json-structures-and-mappings.md index 8d88564bd..d709bf41d 100644 --- a/.claude/skills/mendix/json-structures-and-mappings.md +++ b/.claude/skills/mendix/json-structures-and-mappings.md @@ -10,7 +10,40 @@ A JSON structure defines the schema of a JSON payload. It stores a JSON snippet ### Import Mappings An import mapping converts a JSON string into Mendix entity objects. It maps JSON fields to entity attributes. -### Export Mappings +#### Inherited attributes + +Mendix inheritance is multi-table: all of a parent's attributes are members of the +child, so an entity created with `extends` can map them. Name an inherited +attribute exactly like one of the entity's own — mxcli resolves each to the entity +that **declares** it, which is the reference Studio Pro needs to show the field +mapped. + +```sql +create persistent entity Docs.DocumentBase ( + DocName: String(200), + Confidential: Boolean +); + +create persistent entity Docs.Contract extends Docs.DocumentBase ( + ContractNumber: String(50) +); + +create import mapping Docs.IMM_Contract + with json structure Docs.JSON_Contract +{ + create Docs.Contract { + ContractNumber = contractNumber, -- own + DocName = docName, -- inherited + Confidential = confidential -- inherited + } +}; +``` + +Qualifying an inherited attribute against the entity being mapped instead of its +declaring entity is Mendix **CE1613** "The selected attribute ... no longer +exists", and the field shows unmapped in Studio Pro. + +## Export Mappings An export mapping converts Mendix entity objects into a JSON string. It maps entity attributes to JSON fields. ### Critical: Import and Export Need Different Domain Models diff --git a/.claude/skills/mendix/manage-security.md b/.claude/skills/mendix/manage-security.md index ac6115c22..425a72e4b 100644 --- a/.claude/skills/mendix/manage-security.md +++ b/.claude/skills/mendix/manage-security.md @@ -167,6 +167,58 @@ revoke MyModule.User on MyModule.Customer (write (Email)); revoke MyModule.User on MyModule.Customer (delete); ``` +#### Inherited members + +Mendix inheritance is multi-table: a child adds attributes to its parent's, and +**all** the parent's members belong to the child. Grant them exactly like the +entity's own — `read *` / `write *` cover them too: + +```sql +create persistent entity Docs.DocumentBase ( + DocName: String(200), + Confidential: Boolean +); + +create persistent entity Docs.Contract extends Docs.DocumentBase ( + ContractNumber: String(50) +); + +-- DocName is inherited, ContractNumber is Contract's own — name both the same way +grant Docs.Viewer on Docs.Contract (read (DocName, ContractNumber)); + +-- Attachment inherits the file members from System.FileDocument +create persistent entity Docs.Attachment extends System.FileDocument ( + Category: String(50) +); +grant Docs.Viewer on Docs.Attachment (read (Category, "Name", Size)); +``` + +An access rule must carry an entry for **every** member, own and inherited — +mxcli writes the ones you did not grant with rights `None`. Omitting them is +Mendix **CE0066** "Entity access is out of date", which masks the CE2729 +"No read access to attribute" errors underneath until Studio Pro's *Update +security* is clicked. + +A member name that matches nothing is now an error rather than a silent skip: + +``` +Error: entity Docs.Contract has no member(s) DocNam; grant only names members +of the entity or of an entity it inherits from +``` + +**Exception — user entities.** An entity extending `System.User` is a *user +entity*, and Mendix manages its inherited platform members (`Name`, `Password`, +`Blocked`, …). Those must **not** appear in the rule; listing them is CE0066. +Grant only the entity's own members — mxcli leaves the platform ones out +automatically: + +```sql +create persistent entity Docs.Employee extends System.User ( + EmployeeNo: String(20) +); +grant Docs.Viewer on Docs.Employee (read (EmployeeNo)); -- not Name/Blocked +``` + ### User Roles ```sql diff --git a/.claude/skills/mendix/project-settings.md b/.claude/skills/mendix/project-settings.md index f6a10b2e8..ce601688f 100644 --- a/.claude/skills/mendix/project-settings.md +++ b/.claude/skills/mendix/project-settings.md @@ -30,12 +30,21 @@ alter settings model BeforeShutdownMicroflow = 'Module.MF_Shutdown'; alter settings model HealthCheckMicroflow = 'Module.MF_HealthCheck'; alter settings model HashAlgorithm = 'BCrypt'; alter settings model BcryptCost = 12; -alter settings model JavaVersion = 'Java21'; +alter settings model JavaVersion = 'Java21'; -- or '21'; see note below alter settings model RoundingMode = 'HalfUp'; alter settings model AllowUserMultipleSessions = true; alter settings model ScheduledEventTimeZoneCode = 'Etc/UTC'; ``` +**JavaVersion spelling.** Mendix renamed this property between versions: up to 11.6 +it stores `JavaVersion` = `'Java21'`, from 11.12 it stores `JavaMajorVersion` = +`'21'`. Write either spelling — mxcli reads which one the project uses and stores +the value in that dialect. Getting this wrong is not a cosmetic difference: 11.12 +parses the bare major and rejects the project outright with +`ArgumentOutOfRangeException: majorVersion is an unsupported value: Java21`. +`describe settings` always emits the project's own spelling, so its output replays +cleanly. + ### Modify Configuration Settings ```sql diff --git a/cmd/mxcli/syntax/features_integration.go b/cmd/mxcli/syntax/features_integration.go index 17351ae60..aec81f788 100644 --- a/cmd/mxcli/syntax/features_integration.go +++ b/cmd/mxcli/syntax/features_integration.go @@ -393,7 +393,15 @@ func init() { "show import mappings", "describe import mapping", "with json structure", "find or create", "object handling", }, - Syntax: "SHOW IMPORT MAPPINGS [IN Module];\nDESCRIBE IMPORT MAPPING Module.Name;\nCREATE [OR MODIFY] IMPORT MAPPING Module.Name\n WITH JSON STRUCTURE Module.JsonStruct\n{\n create|find|find or create Module.Entity {\n Attr = jsonField [KEY],\n Assoc/Module.Child = nestedKey { ... }\n }\n};\nDROP IMPORT MAPPING Module.Name;\n\nOR MODIFY: updates mapping in-place, preserves UUID.", + Syntax: "SHOW IMPORT MAPPINGS [IN Module];\nDESCRIBE IMPORT MAPPING Module.Name;\n" + + "CREATE [OR MODIFY] IMPORT MAPPING Module.Name\n WITH JSON STRUCTURE Module.JsonStruct\n{\n" + + " create|find|find or create Module.Entity {\n Attr = jsonField [KEY],\n" + + " Assoc/Module.Child = nestedKey { ... }\n }\n};\nDROP IMPORT MAPPING Module.Name;\n\n" + + "OR MODIFY: updates mapping in-place, preserves UUID.\n\n" + + "Inherited attributes:\n" + + " An entity mapped with EXTENDS can map its inherited attributes too —\n" + + " name them exactly like its own. mxcli resolves each to the entity that\n" + + " declares it, which is what Studio Pro needs to show the field mapped.", Example: "CREATE IMPORT MAPPING Shop.IMM_Order\n WITH JSON STRUCTURE Shop.JSON_Order\n{\n create Shop.Order {\n OrderId = orderId KEY,\n TotalAmount = total\n }\n};\n\n-- Idempotent update\nCREATE OR MODIFY IMPORT MAPPING Shop.IMM_Order\n WITH JSON STRUCTURE Shop.JSON_Order\n{\n find or create Shop.Order {\n OrderId = orderId KEY,\n TotalAmount = total,\n Status = status\n }\n};", SeeAlso: []string{"export-mapping", "json-structure"}, }) diff --git a/cmd/mxcli/syntax/features_security.go b/cmd/mxcli/syntax/features_security.go index 0312bcf0d..9f6eba6f5 100644 --- a/cmd/mxcli/syntax/features_security.go +++ b/cmd/mxcli/syntax/features_security.go @@ -33,8 +33,24 @@ func init() { "entity access", "grant", "revoke", "read", "write", "create", "delete", "xpath", "row-level security", }, - Syntax: "GRANT ON . () [WHERE ''];\nREVOKE ON .;\nREVOKE ON . ();\n\nRights: CREATE, DELETE, READ *, READ (,...), WRITE *, WRITE (,...)", - Example: "GRANT Shop.Admin ON Shop.Customer (CREATE, DELETE, READ *, WRITE *);\nGRANT Shop.User ON Shop.Customer (READ *) WHERE '[Active = true()]';", + Syntax: "GRANT ON . () [WHERE ''];\n" + + "REVOKE ON .;\n" + + "REVOKE ON . ();\n\n" + + "Rights: CREATE, DELETE, READ *, READ (,...), WRITE *, WRITE (,...)\n\n" + + "Inherited members:\n" + + " Mendix inheritance is multi-table — a child adds attributes to its\n" + + " parent's, and ALL the parent's members belong to the child. Name them\n" + + " in a GRANT exactly like the entity's own; READ */WRITE * covers them\n" + + " too. A name that matches no member is an error, not a silent skip.\n\n" + + " Exception: entities extending System.User are user entities, whose\n" + + " platform members (Name, Password, Blocked, ...) Mendix manages. Do not\n" + + " grant those; mxcli leaves them out of the rule automatically.", + Example: "GRANT Shop.Admin ON Shop.Customer (CREATE, DELETE, READ *, WRITE *);\n" + + "GRANT Shop.User ON Shop.Customer (READ *) WHERE '[Active = true()]';\n\n" + + "-- Contract extends DocumentBase: DocName is inherited, ContractNumber is own\n" + + "GRANT Docs.Viewer ON Docs.Contract (READ (DocName, ContractNumber));\n\n" + + "-- Attachment extends System.FileDocument: Name and Size are inherited\n" + + "GRANT Docs.Viewer ON Docs.Attachment (READ (Category, \"Name\", Size));", SeeAlso: []string{"security.module-role", "security.microflow-access"}, }) diff --git a/docs-site/src/language/project-settings.md b/docs-site/src/language/project-settings.md index 5ecd76212..3e5c58103 100644 --- a/docs-site/src/language/project-settings.md +++ b/docs-site/src/language/project-settings.md @@ -32,6 +32,11 @@ ALTER SETTINGS MODEL HashAlgorithm = 'BCrypt'; ALTER SETTINGS MODEL JavaVersion = '17'; ``` +Mendix renamed the Java version property between versions — up to 11.6 it is stored +as `JavaVersion` = `Java21`, from 11.12 as `JavaMajorVersion` = `21`. Write either +spelling (`'17'` or `'Java17'`): mxcli stores the value in whichever dialect the +project already uses. + ### Configuration Settings Server configuration settings like database type, URL, and HTTP port. Each configuration is identified by name (commonly `'default'`): diff --git a/docs-site/src/reference/integration/create-import-mapping.md b/docs-site/src/reference/integration/create-import-mapping.md index 9fbf7e2e0..490c1999c 100644 --- a/docs-site/src/reference/integration/create-import-mapping.md +++ b/docs-site/src/reference/integration/create-import-mapping.md @@ -141,6 +141,32 @@ CREATE OR MODIFY IMPORT MAPPING MyModule.IMM_Pet - Arrays in the JSON sample map directly to child entity objects — there is no intermediate container entity (unlike export mappings). - Import and export of the same JSON typically require different entity structures because of the FK direction difference. +## Inherited attributes + +Mendix inheritance is multi-table: all of a parent's attributes are members of the +child, so an entity declared with `EXTENDS` can map them. Name an inherited +attribute exactly like one of the entity's own; mxcli resolves each to the entity +that **declares** it. + +```sql +CREATE PERSISTENT ENTITY Docs.DocumentBase (DocName: String(200), Confidential: Boolean); +CREATE PERSISTENT ENTITY Docs.Contract EXTENDS Docs.DocumentBase (ContractNumber: String(50)); + +CREATE IMPORT MAPPING Docs.IMM_Contract + WITH JSON STRUCTURE Docs.JSON_Contract +{ + create Docs.Contract { + ContractNumber = contractNumber, + DocName = docName, + Confidential = confidential + } +}; +``` + +Referencing an inherited attribute against the entity being mapped rather than its +declaring entity is Mendix **CE1613** *"The selected attribute ... no longer +exists"*, and Studio Pro shows the field unmapped. + ## See Also [CREATE JSON STRUCTURE](create-json-structure.md), [CREATE EXPORT MAPPING](create-export-mapping.md) diff --git a/docs-site/src/reference/security/grant.md b/docs-site/src/reference/security/grant.md index 7fe165ad1..1f3e0bb0f 100644 --- a/docs-site/src/reference/security/grant.md +++ b/docs-site/src/reference/security/grant.md @@ -125,6 +125,45 @@ GRANT Shop.Viewer ON Shop.Customer (READ (Phone)); -- Result: READ (Name, Email, Phone) ``` +## Inherited members + +Mendix inheritance is multi-table: a child adds attributes to its parent's, and +**all** the parent's members belong to the child. Name an inherited member in a +GRANT exactly like one of the entity's own; `READ *` and `WRITE *` cover them too. + +```sql +CREATE PERSISTENT ENTITY Docs.DocumentBase (DocName: String(200)); +CREATE PERSISTENT ENTITY Docs.Contract EXTENDS Docs.DocumentBase (ContractNumber: String(50)); + +-- DocName inherited, ContractNumber own — no distinction at the call site +GRANT Docs.Viewer ON Docs.Contract (READ (DocName, ContractNumber)); +``` + +An access rule must carry an entry for **every** member, own and inherited. mxcli +writes the members you did not grant with rights `None`; omitting them entirely is +Mendix **CE0066** *"Entity access is out of date"*, which masks the CE2729 +*"No read access to attribute"* errors beneath it until Studio Pro's +**Update security** is clicked. + +A member name that matches nothing is rejected rather than skipped: + +``` +Error: entity Docs.Contract has no member(s) DocNam; grant only names members +of the entity or of an entity it inherits from +``` + +### User entities are the exception + +An entity extending `System.User` is a *user entity*, and Mendix manages its +inherited platform members (`Name`, `Password`, `Blocked`, …). Those must **not** +appear in the access rule — listing them is CE0066. Grant only the entity's own +members; mxcli excludes the platform ones automatically. + +```sql +CREATE PERSISTENT ENTITY Docs.Employee EXTENDS System.User (EmployeeNo: String(20)); +GRANT Docs.Viewer ON Docs.Employee (READ (EmployeeNo)); +``` + ## See Also [REVOKE](revoke.md), [CREATE MODULE ROLE](create-module-role.md), [CREATE USER ROLE](create-user-role.md) diff --git a/docs/01-project/MDL_QUICK_REFERENCE.md b/docs/01-project/MDL_QUICK_REFERENCE.md index 0804ea41a..cc2925ceb 100644 --- a/docs/01-project/MDL_QUICK_REFERENCE.md +++ b/docs/01-project/MDL_QUICK_REFERENCE.md @@ -366,7 +366,7 @@ Nested folders use `/` separator: `'Parent/Child/Grandchild'`. Missing folders a | Revoke nanoflow access | `revoke execute on nanoflow Mod.NF from Mod.Role, ...;` | | | Grant page access | `grant view on page Mod.Page to Mod.Role, ...;` | | | Revoke page access | `revoke view on page Mod.Page from Mod.Role, ...;` | | -| Grant entity access | `grant Mod.Role on Mod.Entity (create, delete, read *, write *);` | Additive — merges with existing | +| Grant entity access | `grant Mod.Role on Mod.Entity (create, delete, read *, write *);` | Additive — merges with existing. Inherited members are named like the entity's own (`read *` covers them); an unknown name is an error. Entities extending `System.User` are the exception — their platform members must not be granted | | Revoke entity access | `revoke Mod.Role on Mod.Entity;` | Full revoke — removes entire rule | | Revoke entity access (partial) | `revoke Mod.Role on Mod.Entity (read (attr));` | Partial — downgrades specific rights | | Set security level | `alter project security level off\|prototype\|production;` | | diff --git a/mdl-examples/bug-tests/703-mapping-inherited-attributes.mdl b/mdl-examples/bug-tests/703-mapping-inherited-attributes.mdl new file mode 100644 index 000000000..87474e738 --- /dev/null +++ b/mdl-examples/bug-tests/703-mapping-inherited-attributes.mdl @@ -0,0 +1,73 @@ +-- Bug #703: import/export mappings silently skipped inherited attributes +-- +-- Mendix inheritance is multi-table: a child adds attributes to its parent's, and +-- all the parent's are members of the child. A mapping element bound to one of +-- them must reference the entity that DECLARES it — `Map703.DocumentBase.DocName`, +-- not `Map703.Contract.DocName`. +-- +-- mxcli prefixed the entity being mapped, unconditionally: +-- +-- attr := def.Attribute +-- if parentEntity != "" && !strings.Contains(attr, ".") { +-- attr = parentEntity + "." + attr // always the CHILD +-- } +-- +-- so every inherited field produced a reference to an attribute that does not +-- exist on that entity. In Studio Pro the field shows as unmapped — the reported +-- symptom — and `mx check` reports CE1613 "The selected attribute ... no longer +-- exists". +-- +-- A second, quieter defect sat alongside it: resolveAttributeType scanned only the +-- entity's own attributes and fell through to a "String" default, so an inherited +-- Boolean or DateTime element got the wrong DataType even once the reference was +-- right. (That function also matched entities by name across every domain model, +-- ignoring the module, so a same-named entity elsewhere could win; it now resolves +-- the module by name.) +-- +-- This is the mapping half of the #765 umbrella. The same declaring-entity rule +-- governs entity access rules (#758) and the change-object writer (#451). +-- +-- Verify: +-- 1. mxcli exec 703-mapping-inherited-attributes.mdl -p app.mpr +-- 2. mxcli docker check -p app.mpr -> 0 errors (no CE1613) +-- 3. Dump the mapping unit: the inherited elements must read +-- Attribute=Map703.DocumentBase.DocName DataType=DataTypes$StringType +-- Attribute=Map703.DocumentBase.Confidential DataType=DataTypes$BooleanType +-- Before the fix both read Map703.Contract.* and both were StringType. +-- 4. Open the mapping in Studio Pro: all three fields are mapped. + +create module Map703; + +create json structure Map703.JSON_Contract +snippet '{"docName": "NDA", "confidential": true, "contractNumber": "C-1"}'; + +-- Parent supplies DocName (String) and Confidential (Boolean). +create persistent entity Map703.DocumentBase ( + DocName: String(200), + Confidential: Boolean +); + +-- Child adds its own; inherits the two above. +create persistent entity Map703.Contract extends Map703.DocumentBase ( + ContractNumber: String(50) +); + +create import mapping Map703.IMM_Contract + with json structure Map703.JSON_Contract +{ + create Map703.Contract { + ContractNumber = contractNumber, + DocName = docName, + Confidential = confidential + } +}; + +create export mapping Map703.EMM_Contract + with json structure Map703.JSON_Contract +{ + Map703.Contract { + contractNumber = ContractNumber, + docName = DocName, + confidential = Confidential + } +}; diff --git a/mdl-examples/bug-tests/758-inherited-member-access.mdl b/mdl-examples/bug-tests/758-inherited-member-access.mdl new file mode 100644 index 000000000..b958b02f3 --- /dev/null +++ b/mdl-examples/bug-tests/758-inherited-member-access.mdl @@ -0,0 +1,70 @@ +-- Bug #758 / #765: security reconciliation stripped inherited members, and GRANT +-- could not put them back +-- +-- Mendix models inheritance across multiple tables: a child adds attributes to the +-- parent's, and ALL the parent's attributes are available on the child. An access +-- rule must therefore carry a MemberAccess entry for every member — own AND +-- inherited — or Mendix reports CE0066 "Entity access is out of date". +-- +-- Two facts established against `mx check`, not inferred: +-- +-- 1. An inherited member's reference is qualified against the entity that +-- DECLARES it. `Sec758.Base.SharedField` validates clean; the child-qualified +-- `Sec758.Item.SharedField` is CE1613 "The selected attribute no longer +-- exists". mxcli wrote the child form. +-- +-- 2. Members inherited from System.User are the exception: entities specialising +-- it are user entities whose platform members Mendix manages. Listing them +-- turns a clean rule into CE0066 — confirmed on Mendix's own +-- Administration.Account and on a fresh specialisation. Every other ancestor's +-- members are required: omitting the six System.FileDocument members from a +-- specialising entity's rule is CE0066 until all are present. +-- +-- What went wrong. Both the GRANT path and reconciliation enumerated only +-- entity.Attributes: +-- +-- * GRANT naming an inherited member produced no entry, and reported success. +-- * ReconcileMemberAccesses — which runs immediately after every GRANT, and on +-- any write touching the module — saw the inherited entry's ancestor +-- qualification, failed to match it against the child's own attributes, and +-- deleted it as stale. So the grant wrote the right thing and the reconcile in +-- the same command removed it. That is why REVOKE + GRANT never repaired a +-- damaged rule. +-- +-- The damage was masked: `mx check` reports CE0066 and stops, hiding the CE2729 +-- "No read access to attribute" cascade underneath until Studio Pro's +-- "Update security" is clicked. +-- +-- Verify: +-- 1. mxcli exec 758-inherited-member-access.mdl -p app.mpr +-- 2. mxcli -p app.mpr -c "describe entity Sec758.Item" +-- -> grant ... (read (OwnField, SharedField)); both members present +-- 3. mxcli docker check -p app.mpr -> 0 errors (no CE0066) +-- 4. Repeat step 3 after any further ALTER on the module: the inherited entry +-- must still be there. + +create module Sec758; +create module role Sec758.Editor; + +create persistent entity Sec758.Base ( + SharedField: String(100) +); + +-- Same-module ancestor: mixed own + inherited members. +create persistent entity Sec758.Item extends Sec758.Base ( + OwnField: String(100) +); + +-- System ancestor whose members ARE required in the child's rule. +create persistent entity Sec758.Attachment extends System.FileDocument ( + "Caption": String(200) +); + +-- User entity: System.User's platform members must NOT appear in the rule. +create persistent entity Sec758.Employee extends System.User ( + EmployeeNo: String(20) +); + +grant Sec758.Editor on Sec758.Item (read (SharedField, OwnField)); +grant Sec758.Editor on Sec758.Attachment (read ("Caption")); +grant Sec758.Editor on Sec758.Employee (read (EmployeeNo)); diff --git a/mdl-examples/bug-tests/759-java-version-value-dialect.mdl b/mdl-examples/bug-tests/759-java-version-value-dialect.mdl new file mode 100644 index 000000000..423684f33 --- /dev/null +++ b/mdl-examples/bug-tests/759-java-version-value-dialect.mdl @@ -0,0 +1,32 @@ +-- Bug #759 (follow-up): the Java version *value* format is version-specific too +-- +-- Symptom: on Mendix 11.12 an `alter settings model JavaVersion = 'Java21'` +-- produced a project mx check refuses to load at all: +-- +-- ERROR: System.ArgumentOutOfRangeException: Specified argument was out of the +-- range of valid values. (Parameter 'majorVersion is an unsupported value: Java21') +-- at Mendix.Modeler.Settings.JavaVersionExtensions.fromString(String majorVersion) +-- at Mendix.Modeler.Settings.RuntimeSettings.get_JavaVersion() +-- +-- Cause: the #759 fix followed the *key* rename ("JavaVersion" up to 11.6, +-- "JavaMajorVersion" from 11.12) but wrote the value through verbatim. The rename +-- changed the value format as well: 11.6 stores the enum member "Java21", 11.12 +-- the bare major "21". Writing "Java21" into JavaMajorVersion is what +-- JavaVersionExtensions.fromString throws on. +-- +-- Before the fix this was worse than a wrong value: unlike the unknown-property +-- shape of the original #759, this one is a *hard* mxbuild failure — the whole +-- project fails to load, so nothing downstream of it can be checked either. +-- +-- Fix: settingsoverlay.JavaVersionValue renders the value in the dialect the +-- stored key expects. Either spelling is accepted on input. +-- +-- Verify: run against an 11.12+ project, then `mx check` — it must load and +-- report no settings error. On an 11.6 project the same script stores 'Java21'. + +-- Both spellings are accepted; both land as the project's own dialect. +alter settings model JavaVersion = 'Java21'; +alter settings model JavaVersion = '21'; + +-- Round-trip: describe emits the project's spelling, which must replay cleanly. +describe settings; diff --git a/mdl-examples/bug-tests/772-xpath-constraint-groups.mdl b/mdl-examples/bug-tests/772-xpath-constraint-groups.mdl new file mode 100644 index 000000000..9655c8fe4 --- /dev/null +++ b/mdl-examples/bug-tests/772-xpath-constraint-groups.mdl @@ -0,0 +1,68 @@ +-- Bug #772: describe silently dropped trailing XPath constraint groups +-- +-- Mendix stores sibling predicate groups concatenated in one XPathConstraint field: +-- +-- [Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit]] +-- [Status != 'Completed'] +-- [CompletionDate = empty] +-- +-- The MDL grammar rule matches ONE bracket group. With the error listeners removed, +-- ANTLR parsed the first group, left the rest on the token stream, and still +-- reported success. enrichXPathConstraintForDescribe read that as a full parse and +-- re-rendered only what came back, so describe emitted: +-- +-- where Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit]; +-- +-- The last two predicates vanished with no warning. That is worse than a crash: the +-- output looks complete while describing a materially LESS RESTRICTIVE query than +-- the project actually contains, which makes correct defensive code read as buggy. +-- +-- Two fixes, defence in depth: +-- 1. ParseXPathConstraint now reports a partial parse as a failure, so a caller +-- falls back to the stored string instead of silently truncating it. +-- 2. visitor.SplitXPathPredicateGroups splits the constraint into top-level groups +-- — tracking nesting depth and quoting, so a nested [..] or a ']' inside a +-- string literal is handled — and each group is enriched and rendered on its +-- own. Splitting on "][" mangled both of those. +-- +-- Verify: +-- 1. mxcli exec 772-xpath-constraint-groups.mdl -p app.mpr +-- 2. mxcli -p app.mpr -c "describe microflow Bug772.MF_GetOpenTasks" +-- All three groups must appear, and Status must render as the qualified +-- enum value Bug772.TaskStatus.Completed (enrichment reaches later groups too). +-- 3. Feed that describe output back through exec: it re-creates the same flow. +-- + +create module Bug772; +create module role Bug772.User; + +create enumeration Bug772.TaskStatus ( + Open 'Open', + Completed 'Completed' +); + +create persistent entity Bug772.TaskGroup ( + Name: String(100), + EndDate: DateTime +); + +create persistent entity Bug772.Task ( + Title: String(100), + Status: Enumeration(Bug772.TaskStatus), + CompletionDate: DateTime +); + +create association Bug772.Task_TaskGroup + from Bug772.Task to Bug772.TaskGroup + type Reference; + +create microflow Bug772.MF_GetOpenTasks ( + EndDateLimit: DateTime +) +returns list of Bug772.Task as $Tasks +begin + retrieve $Tasks from Bug772.Task + where '[Bug772.Task_TaskGroup/Bug772.TaskGroup[EndDate = $EndDateLimit]][Status != Bug772.TaskStatus.Completed][CompletionDate = empty]'; + return $Tasks; +end; +/ diff --git a/mdl-examples/doctype-tests/08-security-examples.mdl b/mdl-examples/doctype-tests/08-security-examples.mdl index 7f6395f16..7ef8b386b 100644 --- a/mdl-examples/doctype-tests/08-security-examples.mdl +++ b/mdl-examples/doctype-tests/08-security-examples.mdl @@ -34,7 +34,8 @@ create module SecTest; create persistent entity SecTest.Customer ( Name: string(200) not null, Email: string(200), - IsActive: boolean default true + IsActive: boolean default true, + Notes: string(500) ); @position(300,100) diff --git a/mdl-examples/doctype-tests/14-project-settings-examples.mdl b/mdl-examples/doctype-tests/14-project-settings-examples.mdl index 25520c5a9..5d59cbac0 100644 --- a/mdl-examples/doctype-tests/14-project-settings-examples.mdl +++ b/mdl-examples/doctype-tests/14-project-settings-examples.mdl @@ -36,6 +36,10 @@ alter settings model AfterStartupMicroflow = 'MyModule.ASU_Startup'; /** * Example 1.2: Configure hash algorithm and Java version + * + * Either spelling of the Java version is accepted -- 'Java21' or '21'. Mendix + * stores it as "JavaVersion" = "Java21" up to 11.6 and as "JavaMajorVersion" = + * "21" from 11.12; mxcli writes whichever dialect the project already uses. */ alter settings model HashAlgorithm = 'BCrypt', JavaVersion = 'Java21'; diff --git a/mdl/backend/modelsdk/attr_ref_owner_test.go b/mdl/backend/modelsdk/attr_ref_owner_test.go new file mode 100644 index 000000000..a28b4b4de --- /dev/null +++ b/mdl/backend/modelsdk/attr_ref_owner_test.go @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import "testing" + +// TestAttrRefBelongsTo pins the distinction reconciliation depends on: only a +// reference qualified against THIS entity names one of its own attributes and can +// be validated from this domain model. An inherited reference carries an +// ancestor's name — possibly from another module or System, neither loaded here — +// and deleting it as "stale" is mendixlabs/mxcli#758. +func TestAttrRefBelongsTo(t *testing.T) { + tests := []struct { + ref string + module string + entity string + want bool + }{ + {"Sec.Item.OwnField", "Sec", "Item", true}, + {"sec.item.OwnField", "Sec", "Item", true}, // Mendix names are case-insensitive + {"Sec.Base.SharedField", "Sec", "Item", false}, + {"System.FileDocument.Name", "Sec", "Attachment", false}, + {"Other.Base.Field", "Sec", "Item", false}, + {"NoDots", "Sec", "Item", false}, + {"", "Sec", "Item", false}, + // A same-named entity in another module is NOT this entity. + {"Other.Item.OwnField", "Sec", "Item", false}, + } + for _, tc := range tests { + if got := attrRefBelongsTo(tc.ref, tc.module, tc.entity); got != tc.want { + t.Errorf("attrRefBelongsTo(%q, %q, %q) = %v, want %v", + tc.ref, tc.module, tc.entity, got, tc.want) + } + } +} diff --git a/mdl/backend/modelsdk/domainmodel_security_write.go b/mdl/backend/modelsdk/domainmodel_security_write.go index 9102579a2..227dca293 100644 --- a/mdl/backend/modelsdk/domainmodel_security_write.go +++ b/mdl/backend/modelsdk/domainmodel_security_write.go @@ -5,6 +5,7 @@ package modelsdkbackend import ( "fmt" "sort" + "strings" "github.com/mendixlabs/mxcli/mdl/backend" "github.com/mendixlabs/mxcli/mdl/types" @@ -412,7 +413,8 @@ func (b *Backend) ReconcileMemberAccesses(unitID model.ID, moduleName string) (i } switch attrRef, assocRef := ma.AttributeQualifiedName(), ma.AssociationQualifiedName(); { case attrRef != "": - if attrSet[attrRef] { + switch { + case attrSet[attrRef]: covAttr[attrRef] = true if calcSet[attrRef] { if r := ma.AccessRights(); r == "ReadWrite" || r == "WriteOnly" { @@ -420,7 +422,21 @@ func (b *Backend) ReconcileMemberAccesses(unitID model.ID, moduleName string) (i changed = true } } - } else { + case !attrRefBelongsTo(attrRef, moduleName, entityName): + // An attribute reference is qualified against the entity that + // DECLARES it, so an inherited member carries an ancestor's name + // rather than this entity's. attrSet holds only this entity's own + // attributes, so every inherited entry looked stale and was + // deleted — silently, on any write touching the module, and + // immediately after the GRANT that had just written it correctly + // (mendixlabs/mxcli#758). The ancestor may live in another module + // or in System, neither loaded here, so an inherited reference + // cannot be validated at this layer at all. Preserve what cannot + // be checked instead of dropping it. + covAttr[attrRef] = true + default: + // Genuinely stale: the reference claims to be this entity's own + // attribute and the entity no longer has it. rule.RemoveMemberAccesses(i) changed = true } @@ -489,3 +505,17 @@ func newMemberAccess(rights, qualifiedName string, isAttr bool) *genDm.MemberAcc assignID(ma) return ma } + +// attrRefBelongsTo reports whether a MemberAccess attribute reference +// ("Module.Entity.Attribute") names one of the given entity's OWN attributes, +// rather than one inherited from an ancestor. +// +// Only an own reference can be validated from a single domain model: an ancestor +// may live in another module or in System, neither of which is loaded here. +func attrRefBelongsTo(attrRef, moduleName, entityName string) bool { + idx := strings.LastIndex(attrRef, ".") + if idx < 0 { + return false + } + return strings.EqualFold(attrRef[:idx], moduleName+"."+entityName) +} diff --git a/mdl/backend/modelsdk/settings_write_759_test.go b/mdl/backend/modelsdk/settings_write_759_test.go index 51a475cca..ba9e6a5ee 100644 --- a/mdl/backend/modelsdk/settings_write_759_test.go +++ b/mdl/backend/modelsdk/settings_write_759_test.go @@ -97,6 +97,60 @@ func TestUpdateProjectSettings_JavaVersionKeyFollowsDocument(t *testing.T) { } } +// TestUpdateProjectSettings_JavaVersionValueMatchesKey covers the follow-up to +// #759: the rename changed the value format along with the key. Mendix 11.12 +// parses JavaMajorVersion with JavaVersionExtensions.fromString, which throws +// ArgumentOutOfRangeException ("majorVersion is an unsupported value: Java21") on +// the 11.6 spelling — so writing the value through verbatim produced a project +// mx check refuses to load. Either spelling on input, the document's own dialect +// on disk. +func TestUpdateProjectSettings_JavaVersionValueMatchesKey(t *testing.T) { + tests := []struct { + name string + storedKey string + seed string + set string + want string + }{ + {"11_12_given_enum_spelling", "JavaMajorVersion", "21", "Java17", "17"}, + {"11_12_given_bare_major", "JavaMajorVersion", "21", "17", "17"}, + {"11_6_given_enum_spelling", "JavaVersion", "Java21", "Java17", "Java17"}, + {"11_6_given_bare_major", "JavaVersion", "Java21", "17", "Java17"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + proj := copyFixture(t) + seedJavaVersionKey(t, proj, tc.storedKey, tc.seed) + + setJavaVersion(t, proj, tc.set) + + if got := readModelSettings(t, proj)[tc.storedKey]; got != tc.want { + t.Errorf("%s = %v, want %q (set %q)", tc.storedKey, got, tc.want, tc.set) + } + }) + } +} + +// setJavaVersion drives one ALTER SETTINGS MODEL JavaVersion through the backend. +func setJavaVersion(t *testing.T, proj, v string) { + t.Helper() + b := New() + if err := b.Connect(proj); err != nil { + t.Fatalf("connect: %v", err) + } + ps, err := b.GetProjectSettings() + if err != nil { + t.Fatalf("GetProjectSettings: %v", err) + } + if ps.Model == nil { + t.Fatal("fixture has no model settings") + } + ps.Model.JavaVersion = v + if err := b.UpdateProjectSettings(ps); err != nil { + t.Fatalf("UpdateProjectSettings: %v", err) + } +} + // seedJavaVersionKey rewrites the fixture's Settings$ModelSettings part so it // carries exactly one Java-version key, standing in for the Mendix version that // spells it that way. diff --git a/mdl/executor/cmd_export_mappings.go b/mdl/executor/cmd_export_mappings.go index 1242a1925..5cbb2aaf4 100644 --- a/mdl/executor/cmd_export_mappings.go +++ b/mdl/executor/cmd_export_mappings.go @@ -365,9 +365,18 @@ func buildExportMappingElementModel(moduleName string, def *ast.ExportMappingEle elem.Kind = "Value" elem.TypeName = "ExportMappings$ValueMappingElement" elem.DataType = resolveAttributeType(parentEntity, def.Attribute, b) + // A member reference is qualified against the entity that DECLARES it, so + // an inherited attribute carries an ancestor's name. Prefixing the entity + // being mapped produced CE1613 "The selected attribute no longer exists" + // and left the field unmapped in Studio Pro (mendixlabs/mxcli#703) — the + // same rule as entity access rules (#758), both under the #765 umbrella. attr := def.Attribute if parentEntity != "" && !strings.Contains(attr, ".") { - attr = parentEntity + "." + attr + if ref, ok := ResolveMemberRef(b, parentEntity, attr); ok { + attr = ref + } else { + attr = parentEntity + "." + attr + } } elem.Attribute = attr // JsonPath already set from JSON structure clone above diff --git a/mdl/executor/cmd_import_mappings.go b/mdl/executor/cmd_import_mappings.go index f31b7ca68..3499a1f33 100644 --- a/mdl/executor/cmd_import_mappings.go +++ b/mdl/executor/cmd_import_mappings.go @@ -341,9 +341,18 @@ func buildImportMappingElementModel(moduleName string, def *ast.ImportMappingEle elem.TypeName = "ImportMappings$ValueMappingElement" elem.DataType = resolveAttributeType(parentEntity, def.Attribute, b) elem.IsKey = def.IsKey + // A member reference is qualified against the entity that DECLARES it, so + // an inherited attribute carries an ancestor's name. Prefixing the entity + // being mapped produced CE1613 "The selected attribute no longer exists" + // and left the field unmapped in Studio Pro (mendixlabs/mxcli#703) — the + // same rule as entity access rules (#758), both under the #765 umbrella. attr := def.Attribute if parentEntity != "" && !strings.Contains(attr, ".") { - attr = parentEntity + "." + attr + if ref, ok := ResolveMemberRef(b, parentEntity, attr); ok { + attr = ref + } else { + attr = parentEntity + "." + attr + } } elem.Attribute = attr } @@ -372,6 +381,18 @@ func resolveAttributeType(entityQN, attrName string, b backend.DomainModelBacken if len(parts) != 2 { return "String" } + // Follow the generalization chain: an inherited attribute is not in the + // entity's own list, and defaulting it to String gave a mapping element the + // wrong DataType (mendixlabs/mxcli#703). Resolving by module name also stops a + // same-named entity in another module being picked up, which the previous + // scan-every-domain-model loop did. + if hb, ok := b.(entityLookupBackend); ok { + if t := ResolveMemberType(hb, entityQN, attrName); t != "" { + return t + } + return "String" + } + dms, err := b.ListDomainModels() if err != nil { return "String" diff --git a/mdl/executor/cmd_microflows_format_action.go b/mdl/executor/cmd_microflows_format_action.go index 88e445230..464e92275 100644 --- a/mdl/executor/cmd_microflows_format_action.go +++ b/mdl/executor/cmd_microflows_format_action.go @@ -429,24 +429,18 @@ func formatAction( // (e.g. Status = 'Open' → Status = Module.OrderStatus.Open) when // the entity is known and we are connected to a project. constraint = enrichXPathConstraintForDescribe(ctx, entityName, constraint) - // XPath may contain multiple predicates like [a][b] or [a]\n[b]. - // Split them and join with MDL 'and' so the parser sees - // separate xpathConstraint nodes. - if strings.HasPrefix(constraint, "[") && strings.HasSuffix(constraint, "]") { - // Split on "][" boundary (possibly separated by \n literals), - // then re-wrap each predicate. - inner := constraint[1 : len(constraint)-1] - // Normalise real newlines between predicates: ]\n[ → ][ - inner = strings.ReplaceAll(inner, "]\n[", "][") - parts := strings.Split(inner, "][") - if len(parts) > 1 { - var wrapped []string - for _, p := range parts { - wrapped = append(wrapped, "["+strings.TrimSpace(p)+"]") - } - constraint = strings.Join(wrapped, "\n ") + // XPath may hold several sibling predicate groups — [a][b], or + // separated by newlines. Emit each on its own line so the parser + // sees separate xpathConstraint nodes. The splitter is nesting- and + // quote-aware: the previous "][" split mangled a group containing a + // nested bracket or a literal with a ']' in it (#772). + if groups := visitor.SplitXPathPredicateGroups(constraint); len(groups) > 0 { + if len(groups) > 1 { + constraint = strings.Join(groups, "\n ") } else { - constraint = parts[0] + // A lone group renders without its outer brackets, matching + // the `where ` form the MDL grammar expects. + constraint = strings.TrimSuffix(strings.TrimPrefix(groups[0], "["), "]") } } stmt += fmt.Sprintf("\n where %s", constraint) @@ -1879,10 +1873,38 @@ func enrichXPathConstraintForDescribe(ctx *ExecContext, entityQN, constraint str if len(enumAttrs) == 0 { return constraint } - expr, ok := visitor.ParseXPathConstraint(constraint) - if !ok || expr == nil { + return enrichXPathGroups(constraint, enumAttrs) +} + +// enrichXPathGroups applies enum enrichment to every top-level predicate group of a +// stored constraint. +// +// Mendix stores sibling groups concatenated (`[a][b][c]`), but the grammar rule +// matches one group. Enriching the whole string at once parsed only the first and +// re-rendered just that, silently discarding the rest — describe then showed a +// materially less restrictive query than the project actually contains +// (mendixlabs/mxcli#772). ParseXPathConstraint now refuses that partial parse, which +// stops the data loss on its own; splitting first is what keeps enrichment working +// for every group rather than only the first. +func enrichXPathGroups(constraint string, enumAttrs map[string]string) string { + groups := visitor.SplitXPathPredicateGroups(constraint) + if len(groups) == 0 { return constraint } - enriched := enrichXPathExprWithEnums(expr, enumAttrs) - return "[" + xpathExprToMDLString(enriched) + "]" + out := make([]string, 0, len(groups)) + for _, g := range groups { + out = append(out, enrichXPathGroup(g, enumAttrs)) + } + return strings.Join(out, "") +} + +// enrichXPathGroup enriches one bracket group, returning it unchanged when it does +// not parse — a group mxcli cannot read is passed through verbatim rather than +// dropped or guessed at. +func enrichXPathGroup(group string, enumAttrs map[string]string) string { + expr, ok := visitor.ParseXPathConstraint(group) + if !ok || expr == nil { + return group + } + return "[" + xpathExprToMDLString(enrichXPathExprWithEnums(expr, enumAttrs)) + "]" } diff --git a/mdl/executor/cmd_security_write.go b/mdl/executor/cmd_security_write.go index beb415345..ffdbb65f8 100644 --- a/mdl/executor/cmd_security_write.go +++ b/mdl/executor/cmd_security_write.go @@ -377,21 +377,30 @@ func execGrantEntityAccess(ctx *ExecContext, s *ast.GrantEntityAccessStmt) error readMemberSet[m] = true } - // Create entries for all entity attributes - for _, attr := range entity.Attributes { + // Create entries for every attribute of the entity's access surface — its own + // AND those inherited through the generalization chain. Enumerating only + // entity.Attributes meant a GRANT naming an inherited member produced no entry + // at all while still reporting success, and left the rule incomplete so Mendix + // reported CE0066 (mendixlabs/mxcli#758). Each reference is qualified against + // the entity that DECLARES the member; qualifying an inherited one against this + // entity is CE1613 "The selected attribute no longer exists". + entityQN := module.Name + "." + s.Entity.Name + members := EntityMembers(ctx, entityQN) + grantedMembers := map[string]bool{} + for _, mem := range members { rights := defaultMemberAccess - if writeMemberSet[attr.Name] { + if writeMemberSet[mem.Name] { rights = "ReadWrite" - } else if readMemberSet[attr.Name] { + } else if readMemberSet[mem.Name] { rights = "ReadOnly" } // Calculated attributes cannot have write rights (CE6592) - isCalculated := attr.Value != nil && attr.Value.Type == "CalculatedValue" - if isCalculated && (rights == "ReadWrite" || rights == "WriteOnly") { + if mem.IsCalculated && (rights == "ReadWrite" || rights == "WriteOnly") { rights = "ReadOnly" } + grantedMembers[mem.Name] = true memberAccesses = append(memberAccesses, types.EntityMemberAccess{ - AttributeRef: module.Name + "." + s.Entity.Name + "." + attr.Name, + AttributeRef: mem.Ref, AccessRights: rights, }) } @@ -407,6 +416,7 @@ func execGrantEntityAccess(ctx *ExecContext, s *ast.GrantEntityAccessStmt) error } else if readMemberSet[assoc.Name] { rights = "ReadOnly" } + grantedMembers[assoc.Name] = true memberAccesses = append(memberAccesses, types.EntityMemberAccess{ AssociationRef: module.Name + "." + assoc.Name, AccessRights: rights, @@ -421,6 +431,7 @@ func execGrantEntityAccess(ctx *ExecContext, s *ast.GrantEntityAccessStmt) error } else if readMemberSet[ca.Name] { rights = "ReadOnly" } + grantedMembers[ca.Name] = true memberAccesses = append(memberAccesses, types.EntityMemberAccess{ AssociationRef: module.Name + "." + ca.Name, AccessRights: rights, @@ -428,6 +439,17 @@ func execGrantEntityAccess(ctx *ExecContext, s *ast.GrantEntityAccessStmt) error } } + // A member named in the GRANT that matched nothing used to be dropped in + // silence — the command reported success and the access simply was not there, + // which is why REVOKE + GRANT could not repair a damaged rule (#758). Name it + // instead. Inherited members now resolve, so anything still unmatched is a typo + // or a member of another entity. + if unknown := unmatchedGrantMembers(readMembers, writeMembers, grantedMembers); len(unknown) > 0 { + return mdlerrors.NewValidationf( + "entity %s has no member(s) %s; grant only names members of the entity or of an entity it inherits from", + entityQN, strings.Join(unknown, ", ")) + } + // Add MemberAccess entries for system associations (owner, changedBy). // When an entity has HasOwner/HasChangedBy, Mendix implicitly adds // System.owner/System.changedBy associations that require MemberAccess. diff --git a/mdl/executor/entity_hierarchy.go b/mdl/executor/entity_hierarchy.go new file mode 100644 index 000000000..f8cf05daa --- /dev/null +++ b/mdl/executor/entity_hierarchy.go @@ -0,0 +1,196 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "sort" + "strings" + + "github.com/mendixlabs/mxcli/mdl/backend" + + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// userEntityBase is the Mendix entity whose specializations are "user entities". +// Its members are managed by the platform (login, blocking, password), so they do +// NOT belong in a specializing entity's access rule — see EntityMembers. +const userEntityBase = "System.User" + +// EntityMember is one member of an entity's access surface: an attribute or an +// association, together with the qualified reference Mendix stores for it. +type EntityMember struct { + Name string // bare member name, as written in GRANT + // Ref is the reference stored in MemberAccess, qualified against the entity + // that DECLARES the member — which is an ancestor for an inherited one. + Ref string + Inherited bool + IsCalculated bool +} + +// EntityMembers returns every member of an entity's access surface: its own +// attributes plus those inherited through the generalization chain, each carrying +// the reference Mendix expects in a MemberAccess entry. +// +// Two rules here are load-bearing, both established against `mx check` rather than +// inferred (mendixlabs/mxcli#758, #765): +// +// 1. An inherited member's reference is qualified against the entity that +// DECLARES it, not the entity carrying the rule. Writing the child's name +// produces CE1613 "The selected attribute ... no longer exists"; writing the +// declaring entity's name validates clean. This is the same rule the +// change-object writer needs (#451). +// +// 2. Members inherited from System.User are excluded. Mendix manages the platform +// members of a user entity, and listing them turns a clean rule into CE0066 — +// verified both on Mendix's own Administration.Account and on a fresh +// specialization. Every other ancestor's members are REQUIRED: omitting the +// six System.FileDocument members from a specializing entity's rule is CE0066 +// until they are all present. +// +// Ancestors that cannot be resolved (module not in the project) stop the walk; the +// members found so far are returned rather than nothing, so a partial model still +// produces a usable rule. +func EntityMembers(ctx *ExecContext, entityQN string) []EntityMember { + if ctx == nil { + return nil + } + return EntityMembersFor(ctx.Backend, entityQN) +} + +// entityLookupBackend is the slice of the backend the generalization walk needs: +// resolve a module by name, then read its domain model. +type entityLookupBackend interface { + backend.ModuleBackend + backend.DomainModelBackend +} + +// EntityMembersFor is EntityMembers against a backend directly, for callers that +// hold one without an ExecContext (the mapping builders). +func EntityMembersFor(b entityLookupBackend, entityQN string) []EntityMember { + var out []EntityMember + seen := map[string]bool{} // cycle guard + claimed := map[string]bool{} // a child's member shadows the ancestor's + + for currentQN, depth := entityQN, 0; currentQN != ""; depth++ { + if seen[currentQN] { + break + } + seen[currentQN] = true + + // Stop before collecting System.User's own members: its specializations are + // user entities, whose platform members Mendix owns. + if depth > 0 && strings.EqualFold(currentQN, userEntityBase) { + break + } + + entity, ok := findEntityByQN(b, currentQN) + if !ok { + break + } + + for _, attr := range entity.Attributes { + if attr == nil || claimed[attr.Name] { + continue + } + claimed[attr.Name] = true + out = append(out, EntityMember{ + Name: attr.Name, + Ref: currentQN + "." + attr.Name, + Inherited: depth > 0, + IsCalculated: attr.Value != nil && attr.Value.Type == "CalculatedValue", + }) + } + currentQN = entity.GeneralizationRef + } + return out +} + +// findEntityByQN resolves a qualified entity name through the backend. The module +// is resolved by name rather than scanning every domain model, so a same-named +// entity in another module cannot be picked up by accident. +func findEntityByQN(b entityLookupBackend, entityQN string) (*domainmodel.Entity, bool) { + if b == nil { + return nil, false + } + parts := strings.SplitN(entityQN, ".", 2) + if len(parts) != 2 { + return nil, false + } + mod, err := b.GetModuleByName(parts[0]) + if err != nil || mod == nil { + return nil, false + } + dm, err := b.GetDomainModel(mod.ID) + if err != nil || dm == nil { + return nil, false + } + entity := dm.FindEntityByName(parts[1]) + if entity == nil { + return nil, false + } + return entity, true +} + +// unmatchedGrantMembers returns the members named in a GRANT that matched no +// attribute or association of the entity, in a stable order. +func unmatchedGrantMembers(readMembers, writeMembers []string, granted map[string]bool) []string { + var unknown []string + seen := map[string]bool{} + for _, list := range [][]string{readMembers, writeMembers} { + for _, name := range list { + if name == "" || granted[name] || seen[name] { + continue + } + seen[name] = true + unknown = append(unknown, name) + } + } + sort.Strings(unknown) + return unknown +} + +// ResolveMemberRef returns the reference Mendix stores for a member of an entity, +// qualified against the entity that DECLARES it — which is an ancestor when the +// member is inherited. Reports false when the entity has no such member. +// +// Qualifying an inherited member against the entity that merely uses it produces +// CE1613 "The selected attribute ... no longer exists": in an access rule +// (mendixlabs/mxcli#758) and equally in an import/export mapping, where Studio Pro +// additionally shows the field as unmapped (#703). +func ResolveMemberRef(b entityLookupBackend, entityQN, memberName string) (string, bool) { + if entityQN == "" || memberName == "" { + return "", false + } + for _, mem := range EntityMembersFor(b, entityQN) { + if mem.Name == memberName { + return mem.Ref, true + } + } + return "", false +} + +// ResolveMemberType returns the data type of an entity's member, following the +// generalization chain. Returns "" when the member cannot be resolved. +func ResolveMemberType(b entityLookupBackend, entityQN, memberName string) string { + if entityQN == "" || memberName == "" { + return "" + } + seen := map[string]bool{} + for currentQN := entityQN; currentQN != ""; { + if seen[currentQN] { + return "" + } + seen[currentQN] = true + entity, ok := findEntityByQN(b, currentQN) + if !ok { + return "" + } + for _, attr := range entity.Attributes { + if attr != nil && attr.Name == memberName && attr.Type != nil { + return attr.Type.GetTypeName() + } + } + currentQN = entity.GeneralizationRef + } + return "" +} diff --git a/mdl/executor/entity_hierarchy_test.go b/mdl/executor/entity_hierarchy_test.go new file mode 100644 index 000000000..5b62767f1 --- /dev/null +++ b/mdl/executor/entity_hierarchy_test.go @@ -0,0 +1,233 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// hierarchyBackend serves three modules: +// +// App.Item extends App.Base (same-module ancestor) +// App.Doc extends System.FileDocument +// App.Employee extends System.User (a user entity) +func hierarchyBackend() *mock.MockBackend { + ids := map[string]model.ID{"App": "mod-app", "System": "mod-system"} + attr := func(name string) *domainmodel.Attribute { + return &domainmodel.Attribute{Name: name} + } + dms := map[model.ID]*domainmodel.DomainModel{ + ids["App"]: {ContainerID: ids["App"], Entities: []*domainmodel.Entity{ + {Name: "Base", Attributes: []*domainmodel.Attribute{attr("SharedField")}}, + {Name: "Item", GeneralizationRef: "App.Base", Attributes: []*domainmodel.Attribute{attr("OwnField")}}, + {Name: "Doc", GeneralizationRef: "System.FileDocument", Attributes: []*domainmodel.Attribute{attr("Caption")}}, + {Name: "Employee", GeneralizationRef: "System.User", Attributes: []*domainmodel.Attribute{attr("EmployeeNo")}}, + }}, + ids["System"]: {ContainerID: ids["System"], Entities: []*domainmodel.Entity{ + {Name: "FileDocument", Attributes: []*domainmodel.Attribute{attr("Name"), attr("Contents")}}, + {Name: "User", Attributes: []*domainmodel.Attribute{attr("Name"), attr("Password")}}, + }}, + } + return &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + GetModuleByNameFunc: func(name string) (*model.Module, error) { + id, ok := ids[name] + if !ok { + return nil, nil + } + return &model.Module{BaseElement: model.BaseElement{ID: id}, Name: name}, nil + }, + GetDomainModelFunc: func(id model.ID) (*domainmodel.DomainModel, error) { return dms[id], nil }, + } +} + +func attrTypeFor(kind string) domainmodel.AttributeType { + if kind == "Boolean" { + return &domainmodel.BooleanAttributeType{} + } + return &domainmodel.StringAttributeType{} +} + +// typedHierarchyBackend adds attribute types and an inherited Boolean, for the +// type-resolution tests. +func typedHierarchyBackend() *mock.MockBackend { + ids := map[string]model.ID{"App": "mod-app", "System": "mod-system"} + typed := func(name, kind string) *domainmodel.Attribute { + return &domainmodel.Attribute{Name: name, Type: attrTypeFor(kind)} + } + dms := map[model.ID]*domainmodel.DomainModel{ + ids["App"]: {ContainerID: ids["App"], Entities: []*domainmodel.Entity{ + {Name: "Base", Attributes: []*domainmodel.Attribute{ + typed("SharedField", "String"), typed("Flag", "Boolean")}}, + {Name: "Item", GeneralizationRef: "App.Base", Attributes: []*domainmodel.Attribute{ + typed("OwnField", "String")}}, + {Name: "Doc", GeneralizationRef: "System.FileDocument", Attributes: []*domainmodel.Attribute{ + typed("Category", "String")}}, + }}, + ids["System"]: {ContainerID: ids["System"], Entities: []*domainmodel.Entity{ + {Name: "FileDocument", Attributes: []*domainmodel.Attribute{typed("Name", "String")}}, + }}, + } + return &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + GetModuleByNameFunc: func(name string) (*model.Module, error) { + id, ok := ids[name] + if !ok { + return nil, nil + } + return &model.Module{BaseElement: model.BaseElement{ID: id}, Name: name}, nil + }, + GetDomainModelFunc: func(id model.ID) (*domainmodel.DomainModel, error) { return dms[id], nil }, + } +} + +func memberRefs(members []EntityMember) []string { + out := make([]string, 0, len(members)) + for _, m := range members { + out = append(out, m.Ref) + } + return out +} + +// TestEntityMembers_InheritedUseDeclaringEntity is the core of +// mendixlabs/mxcli#758 / #765: enumerating only the entity's own attributes meant a +// GRANT naming an inherited member wrote nothing, and reconciliation deleted any +// inherited entry as stale. The reference must be qualified against the entity that +// DECLARES the member — qualifying it against the child is CE1613. +func TestEntityMembers_InheritedUseDeclaringEntity(t *testing.T) { + ctx, _ := newMockCtx(t, withBackend(hierarchyBackend())) + + got := memberRefs(EntityMembers(ctx, "App.Item")) + want := []string{"App.Item.OwnField", "App.Base.SharedField"} + if len(got) != len(want) { + t.Fatalf("EntityMembers(App.Item) = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("member %d = %q, want %q", i, got[i], want[i]) + } + } +} + +// TestEntityMembers_SystemAncestorIncluded: omitting the System.FileDocument +// members from a specializing entity's rule is CE0066 until they are all present, +// so they must be enumerated. +func TestEntityMembers_SystemAncestorIncluded(t *testing.T) { + ctx, _ := newMockCtx(t, withBackend(hierarchyBackend())) + + got := memberRefs(EntityMembers(ctx, "App.Doc")) + for _, want := range []string{"App.Doc.Caption", "System.FileDocument.Name", "System.FileDocument.Contents"} { + found := false + for _, g := range got { + if g == want { + found = true + } + } + if !found { + t.Errorf("EntityMembers(App.Doc) missing %q; got %v", want, got) + } + } +} + +// TestEntityMembers_UserEntityExcludesSystemUser: Mendix manages the platform +// members of a user entity. Listing them turns a clean rule into CE0066 — +// verified against Mendix's own Administration.Account and a fresh specialization. +func TestEntityMembers_UserEntityExcludesSystemUser(t *testing.T) { + ctx, _ := newMockCtx(t, withBackend(hierarchyBackend())) + + got := memberRefs(EntityMembers(ctx, "App.Employee")) + if len(got) != 1 || got[0] != "App.Employee.EmployeeNo" { + t.Errorf("EntityMembers(App.Employee) = %v, want only its own member — "+ + "System.User's platform members must not appear", got) + } +} + +// TestEntityMembers_ChildShadowsAncestor: a child redeclaring an ancestor's +// attribute name must contribute its own reference once, not both. +func TestEntityMembers_ChildShadowsAncestor(t *testing.T) { + ctx, _ := newMockCtx(t, withBackend(hierarchyBackend())) + + // App.Doc declares Caption; System.FileDocument declares Name/Contents. Add a + // shadowing case by checking no member name appears twice. + seen := map[string]bool{} + for _, m := range EntityMembers(ctx, "App.Doc") { + if seen[m.Name] { + t.Errorf("member %q enumerated twice", m.Name) + } + seen[m.Name] = true + } +} + +func TestUnmatchedGrantMembers(t *testing.T) { + granted := map[string]bool{"OwnField": true, "SharedField": true} + got := unmatchedGrantMembers([]string{"SharedField", "Nope"}, []string{"OwnField", "Alsobad"}, granted) + if len(got) != 2 || got[0] != "Alsobad" || got[1] != "Nope" { + t.Errorf("unmatchedGrantMembers = %v, want [Alsobad Nope]", got) + } +} + +// TestResolveMemberRef_DeclaringEntity is the mapping half of the #765 umbrella +// (mendixlabs/mxcli#703): a mapping element bound to an inherited attribute was +// qualified against the entity being mapped, which is CE1613 "The selected +// attribute no longer exists" and leaves the field unmapped in Studio Pro. +func TestResolveMemberRef_DeclaringEntity(t *testing.T) { + b := typedHierarchyBackend() + + tests := []struct { + entity, member, want string + ok bool + }{ + {"App.Item", "OwnField", "App.Item.OwnField", true}, + {"App.Item", "SharedField", "App.Base.SharedField", true}, // inherited + {"App.Doc", "Name", "System.FileDocument.Name", true}, // inherited from System + {"App.Item", "Nonexistent", "", false}, + } + for _, tc := range tests { + got, ok := ResolveMemberRef(b, tc.entity, tc.member) + if ok != tc.ok || got != tc.want { + t.Errorf("ResolveMemberRef(%s, %s) = (%q, %v), want (%q, %v)", + tc.entity, tc.member, got, ok, tc.want, tc.ok) + } + } +} + +// TestResolveMemberType_FollowsChain: an inherited attribute's type was not found +// on the entity itself, so the mapping element defaulted to String — giving an +// inherited Boolean or DateTime the wrong DataType (#703). +func TestResolveMemberType_FollowsChain(t *testing.T) { + b := typedHierarchyBackend() + + if got := ResolveMemberType(b, "App.Item", "OwnField"); got != "String" { + t.Errorf("own attribute type = %q, want String", got) + } + if got := ResolveMemberType(b, "App.Item", "Flag"); got != "Boolean" { + t.Errorf("inherited attribute type = %q, want Boolean — "+ + "defaulting to String is what mistyped mapping elements", got) + } + if got := ResolveMemberType(b, "App.Item", "Nope"); got != "" { + t.Errorf("unresolvable member type = %q, want empty", got) + } +} + +// TestResolveAttributeType_InheritedAttribute covers the mapping call site rather +// than the resolver: resolveAttributeType scanned only the entity's own attributes +// and fell through to its "String" default, so a mapping element bound to an +// inherited Boolean or DateTime got the wrong DataType (mendixlabs/mxcli#703). +func TestResolveAttributeType_InheritedAttribute(t *testing.T) { + b := typedHierarchyBackend() + + if got := resolveAttributeType("App.Item", "OwnField", b); got != "String" { + t.Errorf("own attribute = %q, want String", got) + } + if got := resolveAttributeType("App.Item", "Flag", b); got != "Boolean" { + t.Errorf("inherited attribute = %q, want Boolean — the String default is the bug", got) + } + // An unresolvable member still falls back to the documented default. + if got := resolveAttributeType("App.Item", "Nope", b); got != "String" { + t.Errorf("unresolvable attribute = %q, want the String fallback", got) + } +} diff --git a/mdl/executor/xpath_enrich_772_test.go b/mdl/executor/xpath_enrich_772_test.go new file mode 100644 index 000000000..ed6ae711e --- /dev/null +++ b/mdl/executor/xpath_enrich_772_test.go @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" +) + +// TestEnrichXPathGroups_KeepsEveryGroup is the regression test for +// mendixlabs/mxcli#772. The stored constraint holds three sibling predicate groups; +// enriching the whole string at once parsed only the first, re-rendered just that, +// and dropped the other two — so `describe` showed a materially less restrictive +// query than the project contains, with no warning. +func TestEnrichXPathGroups_KeepsEveryGroup(t *testing.T) { + enumAttrs := map[string]string{"Status": "Reminders.TaskStatus"} + + // Exactly the shape from the issue: a group with a nested bracket, then two + // flat ones, newline-separated. + const stored = "[Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit]]\n" + + "[Status != 'Completed']\n[CompletionDate = empty]" + + got := enrichXPathGroups(stored, enumAttrs) + + for _, want := range []string{ + "Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit]", + "CompletionDate = empty", + } { + if !strings.Contains(got, want) { + t.Errorf("enrichXPathGroups dropped %q\ngot: %s", want, got) + } + } + // The enum comparison must be enriched to a qualified value — in the *second* + // group, which is the one the old code never reached. + if !strings.Contains(got, "Status != Reminders.TaskStatus.Completed") { + t.Errorf("enum enrichment not applied to a later group\ngot: %s", got) + } + if strings.Contains(got, "'Completed'") { + t.Errorf("enum literal left unenriched\ngot: %s", got) + } +} + +// TestEnrichXPathGroups_PassesThroughUnparseable: a constraint mxcli cannot split +// into groups is returned untouched rather than mangled or dropped. +func TestEnrichXPathGroups_PassesThroughUnparseable(t *testing.T) { + enumAttrs := map[string]string{"Status": "Mod.S"} + for _, in := range []string{ + "Status = 'Open'", // no brackets + "[Status = 'Open'", // unbalanced + "[a = 1] and [b = 2]", // content between groups + } { + if got := enrichXPathGroups(in, enumAttrs); got != in { + t.Errorf("enrichXPathGroups(%q) = %q, want it returned verbatim", in, got) + } + } +} + +// TestEnrichXPathGroups_SingleGroupStillEnriched guards the fix from regressing the +// original single-group behaviour. +func TestEnrichXPathGroups_SingleGroupStillEnriched(t *testing.T) { + got := enrichXPathGroups("[Status = 'Open']", map[string]string{"Status": "Mod.S"}) + if got != "[Status = Mod.S.Open]" { + t.Errorf("enrichXPathGroups = %q, want %q", got, "[Status = Mod.S.Open]") + } +} diff --git a/mdl/settingsoverlay/settingsoverlay.go b/mdl/settingsoverlay/settingsoverlay.go index a3a7c52a1..615eafe5f 100644 --- a/mdl/settingsoverlay/settingsoverlay.go +++ b/mdl/settingsoverlay/settingsoverlay.go @@ -134,6 +134,13 @@ func newServerConfiguration(cfg *model.ServerConfiguration, siblings []map[strin } } +// The two spellings of the runtime Java version property. They differ in value +// format as well as in name — see JavaVersionValue. +const ( + JavaMajorVersionKey = "JavaMajorVersion" // Mendix 11.12+, e.g. "21" + JavaVersionEnumKey = "JavaVersion" // Mendix 11.6, e.g. "Java21" +) + // JavaVersionKey returns the storage key a Settings$ModelSettings part uses for // the runtime Java version, or "" when it carries neither. // @@ -146,7 +153,7 @@ func newServerConfiguration(cfg *model.ServerConfiguration, siblings []map[strin // (mendixlabs/mxcli#759). Read the key off the document instead of assuming one, // and never invent a key the document does not already have. func JavaVersionKey(raw map[string]any) string { - for _, k := range []string{"JavaMajorVersion", "JavaVersion"} { + for _, k := range []string{JavaMajorVersionKey, JavaVersionEnumKey} { if _, ok := raw[k]; ok { return k } @@ -165,12 +172,42 @@ func JavaVersion(raw map[string]any) string { return v } -// SetJavaVersion writes the runtime Java version back to the key it was read from. -// A part carrying neither key is left untouched. +// SetJavaVersion writes the runtime Java version back to the key it was read from, +// in the value format that key expects. A part carrying neither key is left +// untouched. func SetJavaVersion(raw map[string]any, v string) { if k := JavaVersionKey(raw); k != "" { - raw[k] = v + raw[k] = JavaVersionValue(k, v) + } +} + +// JavaVersionValue renders a Java version in the form the given storage key holds: +// "JavaVersion" carries the enum member ("Java21"), "JavaMajorVersion" the bare +// major ("21"). +// +// The rename in #759 changed the value format along with the key, and following +// only the key is not enough: Mendix 11.12 parses JavaMajorVersion with +// JavaVersionExtensions.fromString, which throws ArgumentOutOfRangeException +// ("majorVersion is an unsupported value: Java21") on the 11.6 spelling. So an +// `alter settings model JavaVersion = 'Java21'` written verbatim onto an 11.12 +// document produces a project mx check refuses to load. Either spelling is +// accepted on input and stored in the document's own dialect. +// +// A value that is neither spelling — no recognisable major version — is passed +// through untouched, so a typo surfaces as a Mendix error rather than as a +// silently mangled setting. +func JavaVersionValue(key, v string) string { + major := strings.TrimSpace(v) + if len(major) >= 4 && strings.EqualFold(major[:4], "Java") { + major = major[4:] + } + if major == "" || strings.TrimLeft(major, "0123456789") != "" { + return v + } + if key == JavaMajorVersionKey { + return major } + return "Java" + major } // ConstantValues rebuilds a configuration's ConstantValues list, updating each diff --git a/mdl/settingsoverlay/settingsoverlay_test.go b/mdl/settingsoverlay/settingsoverlay_test.go index eef02b4d9..989ab1498 100644 --- a/mdl/settingsoverlay/settingsoverlay_test.go +++ b/mdl/settingsoverlay/settingsoverlay_test.go @@ -365,3 +365,50 @@ func TestJavaVersionKey_FollowsDocument(t *testing.T) { }) } } + +// TestJavaVersionValue_MatchesKeyDialect is the follow-up to +// TestJavaVersionKey_FollowsDocument: the rename changed the value format along +// with the key, so following only the key still produced a project Mendix 11.12 +// refuses to load. Its JavaVersionExtensions.fromString parses JavaMajorVersion as +// a bare major and throws ArgumentOutOfRangeException ("majorVersion is an +// unsupported value: Java21") on the 11.6 spelling. +func TestJavaVersionValue_MatchesKeyDialect(t *testing.T) { + tests := []struct { + key string + in string + want string + }{ + {JavaMajorVersionKey, "Java21", "21"}, + {JavaMajorVersionKey, "21", "21"}, + {JavaMajorVersionKey, "java17", "17"}, + {JavaVersionEnumKey, "Java21", "Java21"}, + {JavaVersionEnumKey, "21", "Java21"}, + {JavaVersionEnumKey, " 17 ", "Java17"}, + // Not a recognisable version: passed through so the typo surfaces as a + // Mendix error rather than as a silently mangled setting. + {JavaMajorVersionKey, "Temurin", "Temurin"}, + {JavaVersionEnumKey, "Java-21", "Java-21"}, + {JavaMajorVersionKey, "", ""}, + } + for _, tc := range tests { + if got := JavaVersionValue(tc.key, tc.in); got != tc.want { + t.Errorf("JavaVersionValue(%q, %q) = %q, want %q", tc.key, tc.in, got, tc.want) + } + } +} + +// TestSetJavaVersion_ConvertsToStoredDialect: one MDL statement must work on both +// Mendix versions, whichever spelling the author used. +func TestSetJavaVersion_ConvertsToStoredDialect(t *testing.T) { + mendix1112 := map[string]any{JavaMajorVersionKey: "21"} + SetJavaVersion(mendix1112, "Java17") + if got := mendix1112[JavaMajorVersionKey]; got != "17" { + t.Errorf("%s = %#v, want %q", JavaMajorVersionKey, got, "17") + } + + mendix116 := map[string]any{JavaVersionEnumKey: "Java21"} + SetJavaVersion(mendix116, "17") + if got := mendix116[JavaVersionEnumKey]; got != "Java17" { + t.Errorf("%s = %#v, want %q", JavaVersionEnumKey, got, "Java17") + } +} diff --git a/mdl/visitor/visitor_xpath_public.go b/mdl/visitor/visitor_xpath_public.go index 270b466eb..7a1d0d043 100644 --- a/mdl/visitor/visitor_xpath_public.go +++ b/mdl/visitor/visitor_xpath_public.go @@ -12,6 +12,14 @@ import ( // [ ] brackets stored by Mendix in the XPathConstraint BSON field — and returns the // AST expression. Returns (nil, false) if the input cannot be parsed (e.g. empty, // malformed, or not starting with '['). +// +// The rule matches a SINGLE bracket group. Mendix stores sibling groups +// concatenated — `[a][b][c]` — and with the error listeners removed ANTLR happily +// parsed the first and left the rest on the stream, returning true. Callers read +// that as "fully parsed" and re-rendered only what came back, silently dropping +// every later group (mendixlabs/mxcli#772). A partial parse is therefore reported +// as a failure so callers fall back to the untouched string; use +// SplitXPathPredicateGroups to handle each group in turn. func ParseXPathConstraint(input string) (ast.Expression, bool) { if input == "" { return nil, false @@ -32,5 +40,9 @@ func ParseXPathConstraint(input string) (ast.Expression, bool) { if xpathExpr == nil { return nil, false } + // Anything left on the stream means the rule consumed only a prefix. + if stream.LA(1) != antlr.TokenEOF { + return nil, false + } return buildXPathExpr(xpathExpr), true } diff --git a/mdl/visitor/xpath_groups.go b/mdl/visitor/xpath_groups.go new file mode 100644 index 000000000..8eb8ca539 --- /dev/null +++ b/mdl/visitor/xpath_groups.go @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import "strings" + +// SplitXPathPredicateGroups splits a stored XPathConstraint into its top-level +// predicate groups, each returned with its enclosing brackets. Mendix concatenates +// sibling groups — `[a][b][c]`, often separated by newlines — and the whole string +// is what the BSON field holds. +// +// Splitting on "][" is not enough: a group may nest brackets of its own +// (`[Mod.Assoc/Mod.Entity[EndDate = $Limit]]`), and a string literal may contain a +// bracket (`[Name = 'a]b']`). Both appear in real projects, and mishandling either +// silently changes the meaning of a query (mendixlabs/mxcli#772). This tracks +// nesting depth and quoting instead. +// +// Returns nil when the input is not a well-formed sequence of bracket groups — +// unbalanced, empty, or with content outside the brackets — so callers can fall +// back to using the string verbatim rather than emit something they invented. +func SplitXPathPredicateGroups(constraint string) []string { + s := strings.TrimSpace(constraint) + if s == "" || !strings.HasPrefix(s, "[") { + return nil + } + + var groups []string + var depth int + var start int + var inQuote bool + + for i := 0; i < len(s); i++ { + c := s[i] + if inQuote { + // Mendix escapes a quote inside a literal by doubling it. + if c == '\'' { + if i+1 < len(s) && s[i+1] == '\'' { + i++ + continue + } + inQuote = false + } + continue + } + switch c { + case '\'': + inQuote = true + case '[': + if depth == 0 { + // Anything between groups must be whitespace only. + if strings.TrimSpace(s[start:i]) != "" { + return nil + } + start = i + } + depth++ + case ']': + depth-- + if depth < 0 { + return nil + } + if depth == 0 { + groups = append(groups, s[start:i+1]) + start = i + 1 + } + } + } + + if depth != 0 || inQuote { + return nil + } + if strings.TrimSpace(s[start:]) != "" { + return nil + } + if len(groups) == 0 { + return nil + } + return groups +} diff --git a/mdl/visitor/xpath_groups_test.go b/mdl/visitor/xpath_groups_test.go new file mode 100644 index 000000000..b7584572c --- /dev/null +++ b/mdl/visitor/xpath_groups_test.go @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "reflect" + "testing" +) + +func TestSplitXPathPredicateGroups(t *testing.T) { + tests := []struct { + name string + in string + want []string + }{ + { + // The constraint from mendixlabs/mxcli#772, verbatim: a group with a + // nested bracket followed by two flat ones, newline-separated. + name: "issue 772 — nested group plus siblings", + in: "[Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit]]\n" + + "[Status != 'Completed']\n[CompletionDate = empty]", + want: []string{ + "[Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit]]", + "[Status != 'Completed']", + "[CompletionDate = empty]", + }, + }, + { + name: "single group", + in: "[Status = 'Open']", + want: []string{"[Status = 'Open']"}, + }, + { + name: "adjacent groups, no separator", + in: "[a = 1][b = 2]", + want: []string{"[a = 1]", "[b = 2]"}, + }, + { + // A "][" split would cut this literal in half. + name: "bracket inside a string literal", + in: "[Name = 'a][b'][Status = 'Open']", + want: []string{"[Name = 'a][b']", "[Status = 'Open']"}, + }, + { + name: "doubled quote escape inside a literal", + in: "[Name = 'it''s]here'][Age = 3]", + want: []string{"[Name = 'it''s]here']", "[Age = 3]"}, + }, + {name: "empty", in: "", want: nil}, + {name: "not bracketed", in: "Status = 'Open'", want: nil}, + {name: "unbalanced open", in: "[a = 1", want: nil}, + {name: "unbalanced close", in: "[a = 1]]", want: nil}, + {name: "content between groups", in: "[a = 1] and [b = 2]", want: nil}, + {name: "unterminated literal", in: "[Name = 'x]", want: nil}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := SplitXPathPredicateGroups(tc.in) + if !reflect.DeepEqual(got, tc.want) { + t.Errorf("SplitXPathPredicateGroups(%q)\n got %#v\nwant %#v", tc.in, got, tc.want) + } + }) + } +} + +// TestParseXPathConstraint_RejectsPartialParse is the core of #772: the rule matches +// one bracket group, and with the error listeners removed ANTLR parsed the first and +// left the rest on the stream while still reporting success. Callers treated that as +// a full parse and re-rendered only what came back. +func TestParseXPathConstraint_RejectsPartialParse(t *testing.T) { + multi := "[Status != 'Completed'][CompletionDate = empty]" + if _, ok := ParseXPathConstraint(multi); ok { + t.Error("ParseXPathConstraint reported success on a multi-group constraint it only partly consumed") + } + // A single group must still parse. + if _, ok := ParseXPathConstraint("[Status != 'Completed']"); !ok { + t.Error("ParseXPathConstraint rejected a single well-formed group") + } +} diff --git a/modelsdk/mpr/security_patch.go b/modelsdk/mpr/security_patch.go index 28e66bacd..c6059ab87 100644 --- a/modelsdk/mpr/security_patch.go +++ b/modelsdk/mpr/security_patch.go @@ -4,6 +4,7 @@ package mpr import ( "fmt" + "strings" "go.mongodb.org/mongo-driver/v2/bson" ) @@ -286,14 +287,28 @@ func secPatchReconcileMemberAccessesDoc(doc bson.D, moduleName string) (bson.D, if attrRef != "" { parts := secSplitQualifiedRef(attrRef) - if parts != "" && attrNames[parts] { + // An attribute reference is qualified against the entity that + // DECLARES it, so an inherited member carries an ancestor's + // name, not this entity's. Reconciliation used to compare only + // the bare member name against this entity's own attributes, + // so every inherited entry looked stale and was deleted — + // silently, and on any write that touched the module + // (mendixlabs/mxcli#758). The ancestor may live in another + // module or in System, neither of which is present in this + // document, so an inherited reference cannot be validated here + // at all: preserve it rather than delete what cannot be + // checked. + if !secRefBelongsToEntity(attrRef, moduleName, entityName) { + filtered = append(filtered, maDoc) + } else if parts != "" && attrNames[parts] { coveredAttrs[parts] = true if calculatedAttrs[parts] { maDoc = secDowngradeCalculatedAttrRights(maDoc) } filtered = append(filtered, maDoc) } else { - // Stale attribute ref (attribute was deleted or renamed). + // Genuinely stale: the reference claims to be this entity's + // own attribute, and the entity no longer has it. changes = append(changes, ReconcileChange{Entity: entityName, Member: parts, Action: "stripped"}) changed = true } @@ -500,3 +515,18 @@ func secStripInvalidAccessRuleProps(doc bson.D) (bson.D, bool) { } return cleaned, stripped } + +// secRefBelongsToEntity reports whether a MemberAccess attribute reference +// ("Module.Entity.Attribute") is qualified against the given entity — i.e. names +// one of its OWN attributes rather than one inherited from an ancestor. +// +// Only an own reference can be validated from this document: an ancestor may live +// in another module or in System, neither of which is loaded here. +func secRefBelongsToEntity(attrRef, moduleName, entityName string) bool { + parts := secSplitByDot(attrRef) + if len(parts) < 3 { + return false + } + owner := strings.Join(parts[:len(parts)-1], ".") + return strings.EqualFold(owner, moduleName+"."+entityName) +} diff --git a/modelsdk/mpr/serialize_services.go b/modelsdk/mpr/serialize_services.go index 7f493b4bc..a2cb94f4d 100644 --- a/modelsdk/mpr/serialize_services.go +++ b/modelsdk/mpr/serialize_services.go @@ -7,6 +7,7 @@ import ( "go.mongodb.org/mongo-driver/v2/bson" + "github.com/mendixlabs/mxcli/mdl/settingsoverlay" "github.com/mendixlabs/mxcli/mdl/types" "github.com/mendixlabs/mxcli/model" ) @@ -191,7 +192,8 @@ func serPSModelSettings(ms *model.ModelSettings, raw map[string]any) map[string] raw["AllowUserMultipleSessions"] = ms.AllowUserMultipleSessions raw["HashAlgorithm"] = ms.HashAlgorithm raw["BcryptCost"] = serPSInt64(ms.BcryptCost) - raw["JavaVersion"] = ms.JavaVersion + // Version-specific key AND value format — see settingsoverlay.JavaVersionValue. + settingsoverlay.SetJavaVersion(raw, ms.JavaVersion) raw["RoundingMode"] = ms.RoundingMode raw["ScheduledEventTimeZoneCode"] = ms.ScheduledEventTimeZoneCode raw["FirstDayOfWeek"] = ms.FirstDayOfWeek diff --git a/sdk/mpr/writer_security.go b/sdk/mpr/writer_security.go index 15b05e148..b40e006f9 100644 --- a/sdk/mpr/writer_security.go +++ b/sdk/mpr/writer_security.go @@ -1472,7 +1472,17 @@ func (w *Writer) ReconcileMemberAccesses(unitID model.ID, moduleName string) (in if attrRef != "" { // Extract attribute name from Module.Entity.AttrName parts := splitQualifiedRef(attrRef) - if parts != "" && attrNames[parts] { + // An inherited member's reference is qualified against the + // entity that DECLARES it, so it does not match this + // entity's own attribute list and used to be deleted as + // stale (mendixlabs/mxcli#758). The ancestor may live in + // another module or in System, neither loaded here, so an + // inherited reference cannot be validated at this layer — + // preserve what cannot be checked. Mirrors the codec engine + // (mdl/backend/modelsdk.attrRefBelongsTo). + if !attrRefBelongsToEntity(attrRef, moduleName, entityName) { + filtered = append(filtered, maDoc) + } else if parts != "" && attrNames[parts] { coveredAttrs[parts] = true // Downgrade write rights on calculated attributes (CE6592) if calculatedAttrs[parts] { @@ -1653,3 +1663,15 @@ func stripInvalidAccessRuleProps(doc bson.D) (bson.D, bool) { // ensure primitive import is used var _ = primitive.Binary{} + +// attrRefBelongsToEntity reports whether a MemberAccess attribute reference +// ("Module.Entity.Attribute") names one of the given entity's OWN attributes, +// rather than one inherited from an ancestor. Only an own reference can be +// validated from a single domain model. +func attrRefBelongsToEntity(attrRef, moduleName, entityName string) bool { + idx := strings.LastIndex(attrRef, ".") + if idx < 0 { + return false + } + return strings.EqualFold(attrRef[:idx], moduleName+"."+entityName) +}