From 41ce03c1b8a197cbb4ba6684044d658e8b399140 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 13:29:17 +0000 Subject: [PATCH] Let MDL set module documentation and the admin user name (#1314) Since #1269 lint reads modules().domain_model_documentation and project_security().admin_user_name, but MDL could write neither: the doc comment before `create or modify module MyFirstModule` is lost ... [MDL089] alter app security has no property "AdminUserName" - The doc comment on `create [or modify] module` is stored as the module's domain model documentation (DomainModels$DomainModel. Documentation; Projects$Module has none). No comment preserves what is stored, `/** */` clears it, and a plain create of an existing module stays a no-op. `describe module` prints it back as that comment. - `alter app security ( AdminUserName: '' )` renames the built-in administrator. The password stays out of MDL (#624). New backend methods SetDomainModelDocumentation and SetProjectAdminUserName change only that property on the stored unit. Checked on a fresh 11.12.5 app: exec, describe round-trip, the catalog column lint reads, and mx check with 0 errors, same as an untouched control. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Uj8k5uUCi1zKZ3Edj8MZ8h --- .../fix-issue/findings/mdl-executor.jsonl | 1 + .../skills/mendix/manage-security/SKILL.md | 4 + cmd/mxcli/syntax/features_security.go | 13 +- docs-site/src/appendixes/quick-reference.md | 1 + docs/01-project/MDL_QUICK_REFERENCE.md | 2 + ...dule-documentation-and-admin-user-name.mdl | 32 ++++ mdl/ast/ast_enumeration.go | 9 + mdl/ast/ast_security.go | 4 + mdl/backend/domainmodel.go | 6 + mdl/backend/mcp/unsupported_gen.go | 10 ++ mdl/backend/mock/backend.go | 2 + mdl/backend/mock/mock_domainmodel.go | 7 + mdl/backend/mock/mock_security.go | 7 + mdl/backend/modelsdk/domainmodel_alter.go | 16 ++ .../issue1314_documentation_admin_test.go | 122 ++++++++++++++ mdl/backend/modelsdk/security_write.go | 12 ++ mdl/backend/modelsdk/unimplemented_gen.go | 25 +++ mdl/backend/security.go | 4 + mdl/executor/cmd_misc.go | 1 + mdl/executor/cmd_module_documentation_test.go | 158 ++++++++++++++++++ mdl/executor/cmd_modules.go | 38 ++++- mdl/executor/cmd_security_write.go | 7 + mdl/executor/documentation_coverage_test.go | 1 + mdl/executor/documentation_preserved_test.go | 8 + mdl/grammar/domains/MDLDomainModel.g4 | 6 +- .../visitor_doc_comment_placement_test.go | 4 +- mdl/visitor/visitor_module.go | 11 +- .../visitor_module_documentation_test.go | 75 +++++++++ mdl/visitor/visitor_security.go | 16 +- 29 files changed, 586 insertions(+), 16 deletions(-) create mode 100644 mdl-examples/bug-tests/1314-module-documentation-and-admin-user-name.mdl create mode 100644 mdl/backend/modelsdk/issue1314_documentation_admin_test.go create mode 100644 mdl/executor/cmd_module_documentation_test.go create mode 100644 mdl/visitor/visitor_module_documentation_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index cc172fdacb..d7e09226af 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -887,3 +887,4 @@ {"area": "mdl/executor", "date": "2026-10-08", "symptom": "`describe microflow` prints `call microflow M.F(…, TraceID = , …)` / `(…, ID = )`, which does not parse (`no viable alternative at input 'TraceID=,'`) — 10 of 1,875 Evora flows, all calls into marketplace modules (GenAICommons, ConversationalUI) whose callee gained a parameter after the caller was written", "cause": "Studio Pro keeps a parameter mapping per callee parameter and stores an argument field left blank as `Argument: \"\"`. The call microflow / call nanoflow printers ran it through describeExpr and printed nothing after `=`. No MDL spelling existed for it: `Param = empty` stores the Mendix value \"empty\" (a different document Studio Pro also writes — `SystemPrompt = empty` sits in the same call) and omitting the argument writes no mapping, so neither round-trips; `Param = nothing` silently stored the text \"nothing\"", "file": "mdl/executor/cmd_microflows_format_action.go", "insight": "An empty stored expression is a third state, distinct from the value `empty` and from absence, and a describer that conflates it with either cannot be Unchanged under ADR-0008 — check the stored bytes of each candidate spelling before choosing one. It is also an ERROR state: mx check 10.24 reports CE0127 \"Missing argument\" for it, which is why the doctype example lists CE0127 as known. The spelling is `Param = nothing` (grammar alternative in callArgument, ast.BlankExpr, renders \"\"), refused by the visitor outside call microflow / call nanoflow. Java actions keep their older, lossy convention (\"\" describes as `empty`, which re-stores \"empty\" when the action resolves) — not changed here", "refs": ["mdl/executor/call_argument_blank_test.go", "mdl-examples/bug-tests/describe-call-blank-argument.mdl"]} {"area": "mdl/executor", "date": "2026-10-08", "symptom": "`describe microflow AgentCommons.PromptToUse_ApplyVariable` (Evora Factory Management, MPR v1, Mendix 10.24) runs past a 5-minute timeout, while the app's other 1,800+ flows — including ones with 4x its McCabe and 5x its activities — describe in seconds. The flow is small: 3 decisions, 2 merges, no loops", "cause": "Not graph complexity. Two of its decisions call a rule (`if AgentCommons.Rule_PromptToUse_ContainsVariable(...)`), and every build asks backend.IsRule for each `if Module.Name(...)`. The canonical describe rebuilds the flow once per derived-layout round (9 here), and the layout read cache did not memoise IsRule. modelsdk IsRule resolved the module of EVERY rule before comparing names; each resolution (moduleNameFor) listed the whole project once, then the modules once per folder level. On MPR v1 every listing reads every unit's contents from SQLite (137 MB), so one IsRule took 53 s on a 29-rule app: 2 splits x 9 rounds x 53 s", "file": "`mdl/backend/modelsdk/microflow.go` (IsRule: compare bare name before resolving the module), `mdl/backend/modelsdk/domainmodel_write.go` (moduleNameFor: list modules once, not per ancestor; moduleNameListings counter), `mdl/executor/cmd_microflows_derived_layout_backend.go` (layoutCheckBackend.IsRule memo); tests TestIsRule_ResolvesOneModule, TestDescribe_DerivedLayoutAsksIsRuleOncePerName; repro `mdl-examples/bug-tests/describe-rule-split-timeout.mdl`", "insight": "When one document is pathologically slow, compare it with the LARGEST documents before theorising about its graph: this one was among the smallest, so the cost had to be a per-construct backend lookup multiplied by rounds, not a traversal. A goroutine dump (SIGQUIT) at a few points showed the same IsRule frame every time, and timing a single call (53 s) settled it before any profile. The multiplier chain is the lesson: per-statement lookup x rebuild rounds x per-candidate project listing x per-ancestor listing — each factor harmless alone, and the derived-layout read cache must memoise EVERY backend read the builder makes, not just the ones the first profile showed. Filter by the cheap key (bare name) before the expensive one (module resolution). Tests count calls/listings, not wall time; reverting gives 32 IsRule calls over 8 rounds (want 1) and 12 project listings for one lookup (want 2). Measured: >5 min (killed) -> 6-10 s, byte-identical output; describe -> exec -> Unchanged, MPR byte-identical"} {"area":"mdl/executor","date":"2026-10-08","symptom":"`mxcli check --references` printed \"Check passed!\" (exit 0) for `grant create on entity MyModule.NoSuchEntity to MyModule.User` and a grant to `MyModule.NoSuchRole`; `exec` wrote the valid grant above them, then failed with \"Error: entity not found: MyModule.NoSuchEntity\" / \"module role not found: MyModule.NoSuchRole\" (reported on v0.25.0, Mendix 11.12.4). Issue mendixlabs/mxcli#1333","cause":"Already fixed before the report reached main: 6731d9df (validateGrantReferences) landed the day the issue was filed, after v0.25.0 was cut. Nothing in the executor changed for #1333.","file":"`mdl/executor/validate_grant_refs.go` (fix, 6731d9df); test added `cmd/mxcli/exec_preflight_grant_refs_test.go`","insight":"**Run the reported script on current main before touching code** — and, when it passes, build the parent of the suspected fixing commit and run it there too: that one control turns \"cannot reproduce\" into \"fixed by \", and `git tag --contains ` tells the reporter which release carries it. The pre-fix binary reproduced the report verbatim (Check passed, exit 0; exec writes grant 1, then entity not found). What was still missing was the issue's actual harm — a valid statement written before the bad one — pinned at the layer it lives in: the existing tests drive the reference check through `agreeCheck`, not `execPreflight`, so nothing asserted exec refuses the MIXED script before writing. The new test does, and fails when the `validateGrantReferences` line in `validate.go` is commented out. Repro `mdl-examples/bug-tests/1333-grant-missing-entity-or-role.mdl`.","refs":["mdl-examples/bug-tests/1333-grant-missing-entity-or-role.mdl","cmd/mxcli/exec_preflight_grant_refs_test.go"]} +{"area": "mdl/executor", "date": "2026-10-08", "symptom": "MDL could not write the two values lint gained readers for in #1269: `/** Sales data */ create or modify module MyFirstModule;` warned \"the doc comment before `create or modify module MyFirstModule` is lost … that statement cannot store documentation, so the comment is ignored [MDL089]\", and `alter app security (AdminUserName: 'appadmin');` failed \"alter app security has no property \\\"AdminUserName\\\"\". Every MDL-built module had empty documentation and the admin was always MxAdmin, so the lint rules over them had no negative case. Issue mendixlabs/mxcli#1314", "cause": "Read-only by design at the time: #1269 added DomainModel.Documentation to the semantic model as read-only, and no backend method wrote it or Security$ProjectSecurity.AdminUserName (gen's SetDocumentation/SetAdminUserName were never called). CreateModuleStmt had no Documentation field, so storesDocumentation() reported its doc comment as detached.", "file": "mdl/visitor/visitor_module.go, mdl/visitor/visitor_security.go (appSecurityKeys), mdl/executor/cmd_modules.go (execCreateModule, setModuleDocumentation, describeModule), mdl/executor/cmd_security_write.go, mdl/backend/modelsdk/domainmodel_alter.go (SetDomainModelDocumentation), mdl/backend/modelsdk/security_write.go (SetProjectAdminUserName)", "fix": "The doc comment on `create [or modify] module` is the domain model's documentation (a Projects$Module has none): new module -> set; existing + `or modify` -> set (`/** */` clears, no comment preserves); plain create of an existing module stays a no-op and says documentation is unchanged. describe module prints it as that comment. `AdminUserName: ''` joins alter app security's property list (string only, non-empty; password stays out per #624).", "insight": "**MDL089 is a capability map, not just a warning**: storesDocumentation() decides by reflecting on the AST for a Documentation field, so adding the field silences MDL089 by itself — the visitor test must assert the field is filled, not only that no detached comment is reported, or a half-wired change passes. When the obvious owner has no property (Projects$Module), look one unit down: the module's documentation is DomainModels$DomainModel.Documentation, which is what lint and Studio Pro's canvas pane show. Verified on a fresh 11.12.5 app: exec, describe round-trip, `select … from catalog.modules`, and `mx check` 0 errors against an untouched control. Repro `mdl-examples/bug-tests/1314-module-documentation-and-admin-user-name.mdl`.", "refs": ["mdl-examples/bug-tests/1314-module-documentation-and-admin-user-name.mdl", "mdl/executor/cmd_module_documentation_test.go", "mdl/visitor/visitor_module_documentation_test.go", "mdl/backend/modelsdk/issue1314_documentation_admin_test.go"]} diff --git a/.claude/skills/mendix/manage-security/SKILL.md b/.claude/skills/mendix/manage-security/SKILL.md index 64a209da39..4a2bf86b27 100644 --- a/.claude/skills/mendix/manage-security/SKILL.md +++ b/.claude/skills/mendix/manage-security/SKILL.md @@ -460,6 +460,10 @@ alter app security ( SecurityLevel: production ); -- Enable/disable demo users alter app security ( EnableDemoUsers: true ); alter app security ( EnableDemoUsers: false ); + +-- Rename the built-in administrator (default MxAdmin). The password is not +-- settable from MDL. +alter app security ( AdminUserName: 'appadmin' ); ``` ### Guest (Anonymous) Access diff --git a/cmd/mxcli/syntax/features_security.go b/cmd/mxcli/syntax/features_security.go index dbd68e34cf..7bf4215f40 100644 --- a/cmd/mxcli/syntax/features_security.go +++ b/cmd/mxcli/syntax/features_security.go @@ -142,25 +142,28 @@ func init() { Register(SyntaxFeature{ Path: "security.project-security", - Summary: "Set project security level, strict mode, demo user and guest access toggles", + Summary: "Set project security level, strict mode, demo user and guest access toggles, admin user name", Keywords: []string{ "project security", "app security", "alter app security", "security level", "prototype", - "production", "off", "strict mode", "SEC005", + "production", "off", "strict mode", "SEC005", "admin user", "AdminUserName", "MxAdmin", }, Syntax: "ALTER APP SECURITY (\n" + " [SecurityLevel: OFF|PROTOTYPE|PRODUCTION,]\n" + " [EnableDemoUsers: TRUE|FALSE,]\n" + " [EnableGuestAccess: TRUE|FALSE,]\n" + " [GuestUserRole: ,]\n" + - " [StrictMode: TRUE|FALSE] -- clears lint rule SEC005\n" + + " [StrictMode: TRUE|FALSE,] -- clears lint rule SEC005\n" + + " [AdminUserName: ''] -- the built-in administrator (default MxAdmin)\n" + ");\n\n" + "-- Set any subset of the properties, in create's ( Key: value ) list. The\n" + "-- clause forms (LEVEL …, DEMO USERS ON|OFF, GUEST ACCESS ON [ROLE r]|OFF,\n" + - "-- STRICT MODE ON|OFF) still run and warn MDL-DEPR133.", + "-- STRICT MODE ON|OFF) still run and warn MDL-DEPR133. The administrator's\n" + + "-- password is not settable from MDL.", Example: "mdl 1;\n" + "ALTER APP SECURITY ( SecurityLevel: PRODUCTION );\n" + "ALTER APP SECURITY ( EnableDemoUsers: FALSE );\n" + - "ALTER APP SECURITY ( StrictMode: TRUE );", + "ALTER APP SECURITY ( StrictMode: TRUE );\n" + + "ALTER APP SECURITY ( AdminUserName: 'appadmin' );", SeeAlso: []string{"security.demo-user", "security.guest-access"}, }) diff --git a/docs-site/src/appendixes/quick-reference.md b/docs-site/src/appendixes/quick-reference.md index 443a7d325d..f787bc8c36 100644 --- a/docs-site/src/appendixes/quick-reference.md +++ b/docs-site/src/appendixes/quick-reference.md @@ -226,6 +226,7 @@ Nested folders use `/` separator: `'Parent/Child/Grandchild'`. Missing folders a | Set security level | `ALTER APP SECURITY ( SecurityLevel: OFF\|PROTOTYPE\|PRODUCTION );` | | | Toggle demo users | `ALTER APP SECURITY ( EnableDemoUsers: TRUE\|FALSE );` | | | Toggle guest access | `ALTER APP SECURITY ( EnableGuestAccess: TRUE, GuestUserRole: UserRole );` / `( EnableGuestAccess: FALSE )` | Anonymous users; role required (CE0133) | +| Rename admin user | `ALTER APP SECURITY ( AdminUserName: 'appadmin' );` | Default `MxAdmin`; the password is not settable from MDL | | Create demo user | `CREATE DEMO USER 'name' ( Password: 'pass', [Entity: Module.Entity,] UserRoles: (UserRole, ...) );` | The clause form `PASSWORD … (…)` warns MDL-DEPR137 | | Drop demo user | `DROP DEMO USER [IF EXISTS] 'name';` | `IF EXISTS` makes a cleanup script re-runnable | diff --git a/docs/01-project/MDL_QUICK_REFERENCE.md b/docs/01-project/MDL_QUICK_REFERENCE.md index f5b0ffa091..8a70b645ce 100644 --- a/docs/01-project/MDL_QUICK_REFERENCE.md +++ b/docs/01-project/MDL_QUICK_REFERENCE.md @@ -63,6 +63,7 @@ create persistent entity Module.Photo ( | Show modules | `list modules;` | List all modules | | Describe module | `describe module ModuleName;` | All contents (entities, microflows, pages, etc.) | | Create module | `create module ModuleName;` | | +| Module documentation | `/** text */ create or modify module ModuleName;` | Stored as the module's **domain model** documentation (a module has none of its own) — what lint reads as `modules().domain_model_documentation`. `describe module` prints it the same way. No comment leaves it as stored; `/** */` clears it; a plain `create module` of an existing module changes nothing | | Drop module | `drop module [if exists] ModuleName;` | | | Rename module | `rename module OldName to NewName;` | Updates all qualified name references | @@ -710,6 +711,7 @@ Nested folders use `/` separator: `'Parent/Child/Grandchild'`. Missing folders a | Toggle demo users | `alter app security ( EnableDemoUsers: true\|false );` | Several keys may go in one list | | Enable guest access | `alter app security ( EnableGuestAccess: true, GuestUserRole: UserRole );` | Anonymous users. The role is what visitors get — its entity access is the public surface. Mendix fails the build without one (CE0133), so `on` is refused unless a role is given or already stored. mxcli validates the role exists; Mendix does not | | Disable guest access | `alter app security ( EnableGuestAccess: false );` | Keeps the stored role, so re-enabling needs no `GuestUserRole` | +| Rename admin user | `alter app security ( AdminUserName: 'appadmin' );` | The built-in administrator (default `MxAdmin`), read by lint as `project_security().admin_user_name`. Its password is not settable from MDL | | Create demo user | `create demo user 'name' ( Password: 'pass', [Entity: Module.Entity,] UserRoles: (UserRole, ...) );` | The clause form `password … (…)` warns MDL-DEPR137 | | Drop demo user | `drop demo user [if exists] 'name';` | `if exists` makes a cleanup script re-runnable | | Update security | `update security [[in] Module];` | Re-syncs access rules with their domain model — Studio Pro's **Update security** button, headless. Repairs **CE0066** "Entity access is out of date", which a model authored elsewhere can carry (a module imported or updated outside Studio Pro). Not needed after mxcli's own writes: every write path reconciles as it writes. Writes nothing when the rules already match, and skips `System` | diff --git a/mdl-examples/bug-tests/1314-module-documentation-and-admin-user-name.mdl b/mdl-examples/bug-tests/1314-module-documentation-and-admin-user-name.mdl new file mode 100644 index 0000000000..0122cb0fcd --- /dev/null +++ b/mdl-examples/bug-tests/1314-module-documentation-and-admin-user-name.mdl @@ -0,0 +1,32 @@ +mdl 1; +-- mendixlabs/mxcli#1314: since #1269 lint reads +-- modules().domain_model_documentation and project_security().admin_user_name, +-- and MDL could write neither, so the rules over them had no negative case in +-- an MDL-built app. Reported: +-- +-- line 2: the doc comment before `create or modify module MyFirstModule` is lost +-- … that statement cannot store documentation, so the comment is ignored [MDL089] +-- line 2: alter app security has no property "AdminUserName"; … +-- +-- A Mendix module has no documentation property; its domain model +-- (DomainModels$DomainModel.Documentation) does, and that is what the doc +-- comment on `create module` now sets. `describe module` prints it back as +-- that comment. A comment that is absent leaves the stored text alone; `/** */` +-- clears it. A plain `create module` of a module that exists stays a no-op. +-- +-- The admin password stays out of MDL (mendixlabs/mxcli#624). +-- +-- Verify: +-- +-- mxcli exec 1314-module-documentation-and-admin-user-name.mdl -p app.mpr +-- mxcli -p app.mpr -c "describe module Bug1314Sales" -- prints the /** … */ +-- mxcli -p app.mpr -c "show project security" -- Admin User: appadmin +-- mxcli docker check -p app.mpr +-- +-- Then perturb: drop the doc comment and re-run — describe still prints it +-- (absent preserves); replace it with `/** */` — it is gone. + +/** Sales data, per region */ +create or modify module Bug1314Sales; + +alter app security ( AdminUserName: 'appadmin' ); diff --git a/mdl/ast/ast_enumeration.go b/mdl/ast/ast_enumeration.go index 8e0f27c0e3..64c229d25d 100644 --- a/mdl/ast/ast_enumeration.go +++ b/mdl/ast/ast_enumeration.go @@ -17,6 +17,15 @@ type EnumValue struct { type CreateModuleStmt struct { CreateGuard // `create … if not exists` (ako/mxcli#731) Name string + // CreateOrModify is `create or modify module`: on a module that exists, the + // statement's documentation is applied rather than the statement being a + // no-op. + CreateOrModify bool + // Documentation is the doc comment, stored as the module's domain model + // documentation: a Mendix module has none of its own, its + // DomainModels$DomainModel does (mendixlabs/mxcli#1314). + Documentation string + DocumentationSet bool // see mendixlabs/mxcli#1018: absent preserves, empty clears } func (s *CreateModuleStmt) isStatement() {} diff --git a/mdl/ast/ast_security.go b/mdl/ast/ast_security.go index ea45c91751..0a110cb449 100644 --- a/mdl/ast/ast_security.go +++ b/mdl/ast/ast_security.go @@ -210,6 +210,10 @@ type AlterProjectSecurityStmt struct { // A pointer, so "the statement said nothing about it" is distinguishable // from "the statement asked for off". StrictModeEnabled *bool + // AdminUserName renames the built-in administrator account (MxAdmin by + // default). Empty means "keep what is stored". Its password is deliberately + // not settable from MDL (mendixlabs/mxcli#624). + AdminUserName string } func (s *AlterProjectSecurityStmt) isStatement() {} diff --git a/mdl/backend/domainmodel.go b/mdl/backend/domainmodel.go index dab26fb4b6..3b1d28eaaa 100644 --- a/mdl/backend/domainmodel.go +++ b/mdl/backend/domainmodel.go @@ -27,6 +27,12 @@ type DomainModelBackend interface { // deleted in the first place. SetDomainModelAnnotations(domainModelID model.ID, annotations []*domainmodel.Annotation) error + // SetDomainModelDocumentation sets the domain model's own documentation — + // the only documentation a module has (mendixlabs/mxcli#1314). Separate + // from UpdateDomainModel for the same reason as annotations: that one + // carries the stored value through untouched. + SetDomainModelDocumentation(domainModelID model.ID, documentation string) error + // Entities CreateEntity(domainModelID model.ID, entity *domainmodel.Entity) error UpdateEntity(domainModelID model.ID, entity *domainmodel.Entity) error diff --git a/mdl/backend/mcp/unsupported_gen.go b/mdl/backend/mcp/unsupported_gen.go index cdbeef6302..7197c9863d 100644 --- a/mdl/backend/mcp/unsupported_gen.go +++ b/mdl/backend/mcp/unsupported_gen.go @@ -1152,6 +1152,16 @@ func (unsupportedBackend) SetDomainModelAnnotations(_ model.ID, _ []*domainmodel return } +func (unsupportedBackend) SetDomainModelDocumentation(_ model.ID, _ string) (err0 error) { + err0 = errUnsupported("SetDomainModelDocumentation") + return +} + +func (unsupportedBackend) SetProjectAdminUserName(_ model.ID, _ string) (err0 error) { + err0 = errUnsupported("SetProjectAdminUserName") + return +} + func (unsupportedBackend) SetProjectDemoUsersEnabled(_ model.ID, _ bool) (err0 error) { err0 = errUnsupported("SetProjectDemoUsersEnabled") return diff --git a/mdl/backend/mock/backend.go b/mdl/backend/mock/backend.go index df026b7f92..4d69899379 100644 --- a/mdl/backend/mock/backend.go +++ b/mdl/backend/mock/backend.go @@ -65,6 +65,7 @@ type MockBackend struct { GetDomainModelByIDFunc func(id model.ID) (*domainmodel.DomainModel, error) UpdateDomainModelFunc func(dm *domainmodel.DomainModel) error SetDomainModelAnnotationsFunc func(domainModelID model.ID, annotations []*domainmodel.Annotation) error + SetDomainModelDocumentationFunc func(domainModelID model.ID, documentation string) error CreateEntityFunc func(domainModelID model.ID, entity *domainmodel.Entity) error UpdateEntityFunc func(domainModelID model.ID, entity *domainmodel.Entity) error DeleteEntityFunc func(domainModelID model.ID, entityID model.ID) error @@ -155,6 +156,7 @@ type MockBackend struct { SetProjectSecurityLevelFunc func(unitID model.ID, level string) error SetProjectDemoUsersEnabledFunc func(unitID model.ID, enabled bool) error SetProjectStrictModeFunc func(unitID model.ID, enabled bool) error + SetProjectAdminUserNameFunc func(unitID model.ID, name string) error SetProjectGuestAccessFunc func(unitID model.ID, enabled bool, guestUserRole string) error AddUserRoleFunc func(unitID model.ID, name string, moduleRoles []string, manageAllRoles bool) error AlterUserRoleModuleRolesFunc func(unitID model.ID, userRoleName string, add bool, moduleRoles []string) error diff --git a/mdl/backend/mock/mock_domainmodel.go b/mdl/backend/mock/mock_domainmodel.go index 6d10e71df4..2dc8a4030b 100644 --- a/mdl/backend/mock/mock_domainmodel.go +++ b/mdl/backend/mock/mock_domainmodel.go @@ -45,6 +45,13 @@ func (m *MockBackend) SetDomainModelAnnotations(domainModelID model.ID, annotati return fmt.Errorf("MockBackend.SetDomainModelAnnotations not configured") } +func (m *MockBackend) SetDomainModelDocumentation(domainModelID model.ID, documentation string) error { + if m.SetDomainModelDocumentationFunc != nil { + return m.SetDomainModelDocumentationFunc(domainModelID, documentation) + } + return fmt.Errorf("MockBackend.SetDomainModelDocumentation not configured") +} + func (m *MockBackend) CreateEntity(domainModelID model.ID, entity *domainmodel.Entity) error { if m.CreateEntityFunc != nil { return m.CreateEntityFunc(domainModelID, entity) diff --git a/mdl/backend/mock/mock_security.go b/mdl/backend/mock/mock_security.go index 9631af3454..a8ecea0fbd 100644 --- a/mdl/backend/mock/mock_security.go +++ b/mdl/backend/mock/mock_security.go @@ -32,6 +32,13 @@ func (m *MockBackend) SetProjectDemoUsersEnabled(unitID model.ID, enabled bool) return nil } +func (m *MockBackend) SetProjectAdminUserName(unitID model.ID, name string) error { + if m.SetProjectAdminUserNameFunc != nil { + return m.SetProjectAdminUserNameFunc(unitID, name) + } + return fmt.Errorf("MockBackend.SetProjectAdminUserName not configured") +} + func (m *MockBackend) SetProjectStrictMode(unitID model.ID, enabled bool) error { if m.SetProjectStrictModeFunc != nil { return m.SetProjectStrictModeFunc(unitID, enabled) diff --git a/mdl/backend/modelsdk/domainmodel_alter.go b/mdl/backend/modelsdk/domainmodel_alter.go index 16d412ef64..d6687acd10 100644 --- a/mdl/backend/modelsdk/domainmodel_alter.go +++ b/mdl/backend/modelsdk/domainmodel_alter.go @@ -685,6 +685,22 @@ func (b *Backend) SetDomainModelAnnotations(domainModelID model.ID, annotations return b.persistDM(domainModelID, gdm) } +// SetDomainModelDocumentation sets DomainModels$DomainModel.Documentation, the +// domain model's own documentation and the only documentation a module has +// (mendixlabs/mxcli#1314). It loads the stored unit and changes that one +// property, so entities, associations and annotations pass through as stored. +func (b *Backend) SetDomainModelDocumentation(domainModelID model.ID, documentation string) error { + if b.writer == nil { + return fmt.Errorf("SetDomainModelDocumentation: not connected for writing") + } + gdm, err := b.loadDomainModelGen(domainModelID) + if err != nil { + return err + } + gdm.SetDocumentation(documentation) + return b.persistDM(domainModelID, gdm) +} + // entityIsExternal reports whether a stored entity is an external (OData) one, // whose attributes carry mapped remote values rather than plain Mendix types. // diff --git a/mdl/backend/modelsdk/issue1314_documentation_admin_test.go b/mdl/backend/modelsdk/issue1314_documentation_admin_test.go new file mode 100644 index 0000000000..40827f4bdb --- /dev/null +++ b/mdl/backend/modelsdk/issue1314_documentation_admin_test.go @@ -0,0 +1,122 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "testing" + + "go.mongodb.org/mongo-driver/bson" + + "github.com/mendixlabs/mxcli/model" +) + +// storedString reads a top-level string property from a unit as stored. +func storedString(t *testing.T, b *Backend, id model.ID, key string) string { + t.Helper() + raw, err := b.GetRawUnitBytes(id) + if err != nil { + t.Fatalf("GetRawUnitBytes: %v", err) + } + var d bson.D + if err := bson.Unmarshal(raw, &d); err != nil { + t.Fatalf("unmarshal: %v", err) + } + for _, e := range d { + if e.Key == key { + s, _ := e.Value.(string) + return s + } + } + t.Fatalf("unit %s has no %s property", id, key) + return "" +} + +// mendixlabs/mxcli#1314: the domain model's Documentation is written on the +// DomainModels$DomainModel unit — the property #1269 reads — and survives a +// reconnect. +func TestSetDomainModelDocumentation(t *testing.T) { + proj := copyFixture(t) + b := New() + if err := b.Connect(proj); err != nil { + t.Fatalf("connect: %v", err) + } + dms, err := b.ListDomainModels() + if err != nil || len(dms) == 0 { + t.Fatalf("ListDomainModels: %v (%d)", err, len(dms)) + } + var dmID model.ID + for _, dm := range dms { + if dm.Documentation == "" && dm.ContainerID != "" { + if _, err := b.loadDomainModelGen(dm.ID); err == nil { + dmID = dm.ID + break + } + } + } + if dmID == "" { + t.Fatal("fixture has no stored, undocumented domain model") + } + // Control: the fixture starts undocumented. + if got := storedString(t, b, dmID, "Documentation"); got != "" { + t.Fatalf("fixture domain model already documented: %q", got) + } + + if err := b.SetDomainModelDocumentation(dmID, "Sales data"); err != nil { + t.Fatalf("SetDomainModelDocumentation: %v", err) + } + _ = b.Disconnect() + + b = New() + if err := b.Connect(proj); err != nil { + t.Fatalf("reconnect: %v", err) + } + t.Cleanup(func() { _ = b.Disconnect() }) + if got := storedString(t, b, dmID, "Documentation"); got != "Sales data" { + t.Errorf("stored Documentation = %q, want %q", got, "Sales data") + } + dm, err := b.GetDomainModelByID(dmID) + if err != nil || dm.Documentation != "Sales data" { + t.Errorf("read back %+v (%v), want Documentation %q", dm, err, "Sales data") + } +} + +// mendixlabs/mxcli#1314: AdminUserName is written on Security$ProjectSecurity +// and nothing else about the administrator changes. +func TestSetProjectAdminUserName(t *testing.T) { + proj := copyFixture(t) + b := New() + if err := b.Connect(proj); err != nil { + t.Fatalf("connect: %v", err) + } + ps, err := b.GetProjectSecurity() + if err != nil { + t.Fatalf("GetProjectSecurity: %v", err) + } + // Control: the fixture has the default administrator name. + before := storedString(t, b, ps.ID, "AdminUserName") + if before == "" || before == "appadmin" { + t.Fatalf("fixture AdminUserName = %q; the control does not hold", before) + } + pwBefore := storedString(t, b, ps.ID, "AdminPassword") + + if err := b.SetProjectAdminUserName(ps.ID, "appadmin"); err != nil { + t.Fatalf("SetProjectAdminUserName: %v", err) + } + _ = b.Disconnect() + + b = New() + if err := b.Connect(proj); err != nil { + t.Fatalf("reconnect: %v", err) + } + t.Cleanup(func() { _ = b.Disconnect() }) + if got := storedString(t, b, ps.ID, "AdminUserName"); got != "appadmin" { + t.Errorf("stored AdminUserName = %q, want appadmin", got) + } + if got := storedString(t, b, ps.ID, "AdminPassword"); got != pwBefore { + t.Errorf("AdminPassword changed: %q -> %q", pwBefore, got) + } + got, err := b.GetProjectSecurity() + if err != nil || got.AdminUserName != "appadmin" { + t.Errorf("read back AdminUserName %q (%v), want appadmin", got.AdminUserName, err) + } +} diff --git a/mdl/backend/modelsdk/security_write.go b/mdl/backend/modelsdk/security_write.go index 38c29921ce..449a893143 100644 --- a/mdl/backend/modelsdk/security_write.go +++ b/mdl/backend/modelsdk/security_write.go @@ -211,6 +211,18 @@ func (b *Backend) SetProjectStrictMode(unitID model.ID, enabled bool) error { return b.persistUnit(unitID, ps) } +// SetProjectAdminUserName renames the built-in administrator account +// (Security$ProjectSecurity.AdminUserName, MxAdmin by default). The password +// is left as stored: MDL does not set it (mendixlabs/mxcli#624). +func (b *Backend) SetProjectAdminUserName(unitID model.ID, name string) error { + ps, err := b.loadProjectSecurityGen(unitID) + if err != nil { + return err + } + ps.SetAdminUserName(name) + return b.persistUnit(unitID, ps) +} + // SetProjectGuestAccess toggles anonymous (guest) access. An empty // guestUserRole leaves the stored role alone, so turning access off and back on // does not lose it. diff --git a/mdl/backend/modelsdk/unimplemented_gen.go b/mdl/backend/modelsdk/unimplemented_gen.go index 3a07f923cb..aef3088b25 100644 --- a/mdl/backend/modelsdk/unimplemented_gen.go +++ b/mdl/backend/modelsdk/unimplemented_gen.go @@ -172,6 +172,10 @@ func (unimplemented) CreateMessageDefinitionCollection(_ *model.MessageDefinitio return errUnimplemented("CreateMessageDefinitionCollection") } +func (unimplemented) CreateMessageDefinitionDocument(_ *model.MessageDefinitionDocument) error { + return errUnimplemented("CreateMessageDefinitionDocument") +} + func (unimplemented) CreateMicroflow(_ *microflows.Microflow) error { return errUnimplemented("CreateMicroflow") } @@ -333,6 +337,10 @@ func (unimplemented) DeleteMessageDefinitionCollection(_ string) error { return errUnimplemented("DeleteMessageDefinitionCollection") } +func (unimplemented) DeleteMessageDefinitionDocument(_ string) error { + return errUnimplemented("DeleteMessageDefinitionDocument") +} + func (unimplemented) DeleteMicroflow(_ model.ID) error { return errUnimplemented("DeleteMicroflow") } @@ -721,6 +729,11 @@ func (unimplemented) ListMessageDefinitionCollections() ([]*model.MessageDefinit return r0, errUnimplemented("ListMessageDefinitionCollections") } +func (unimplemented) ListMessageDefinitionDocuments() ([]*model.MessageDefinitionDocument, error) { + var r0 []*model.MessageDefinitionDocument + return r0, errUnimplemented("ListMessageDefinitionDocuments") +} + func (unimplemented) ListMicroflows() ([]*microflows.Microflow, error) { var r0 []*microflows.Microflow return r0, errUnimplemented("ListMicroflows") @@ -1033,6 +1046,14 @@ func (unimplemented) SetDomainModelAnnotations(_ model.ID, _ []*domainmodel.Anno return errUnimplemented("SetDomainModelAnnotations") } +func (unimplemented) SetDomainModelDocumentation(_ model.ID, _ string) error { + return errUnimplemented("SetDomainModelDocumentation") +} + +func (unimplemented) SetProjectAdminUserName(_ model.ID, _ string) error { + return errUnimplemented("SetProjectAdminUserName") +} + func (unimplemented) SetProjectDemoUsersEnabled(_ model.ID, _ bool) error { return errUnimplemented("SetProjectDemoUsersEnabled") } @@ -1153,6 +1174,10 @@ func (unimplemented) UpdateMessageDefinitionCollection(_ *model.MessageDefinitio return errUnimplemented("UpdateMessageDefinitionCollection") } +func (unimplemented) UpdateMessageDefinitionDocument(_ *model.MessageDefinitionDocument) error { + return errUnimplemented("UpdateMessageDefinitionDocument") +} + func (unimplemented) UpdateMicroflow(_ *microflows.Microflow) error { return errUnimplemented("UpdateMicroflow") } diff --git a/mdl/backend/security.go b/mdl/backend/security.go index 2cdd00429d..0112ab37fb 100644 --- a/mdl/backend/security.go +++ b/mdl/backend/security.go @@ -30,6 +30,10 @@ type ProjectSecurityBackend interface { // READ (it is what lint rule SEC005 reports on) and could not write, so the // rule had no remedy short of Studio Pro (ako/mxcli#526). SetProjectStrictMode(unitID model.ID, enabled bool) error + // SetProjectAdminUserName renames the built-in administrator account + // (Security$ProjectSecurity.AdminUserName), which lint reads as + // project_security().admin_user_name (mendixlabs/mxcli#1314). + SetProjectAdminUserName(unitID model.ID, name string) error // SetProjectGuestAccess toggles anonymous access. An empty guestUserRole // leaves the stored role untouched — the caller is responsible for having // established that a role exists, because Mendix raises CE0133 on guest diff --git a/mdl/executor/cmd_misc.go b/mdl/executor/cmd_misc.go index 01168c2d9b..f73b222839 100644 --- a/mdl/executor/cmd_misc.go +++ b/mdl/executor/cmd_misc.go @@ -251,6 +251,7 @@ Security - Project Settings: alter app security ( SecurityLevel: off|prototype|production ); alter app security ( EnableDemoUsers: true|false, StrictMode: true|false ); alter app security ( EnableGuestAccess: true|false [, GuestUserRole: ] ); + alter app security ( AdminUserName: '' ); create demo user 'name' ( Password: 'pass', UserRoles: (UserRole [, ...]) ); drop demo user 'name'; diff --git a/mdl/executor/cmd_module_documentation_test.go b/mdl/executor/cmd_module_documentation_test.go new file mode 100644 index 0000000000..759542f3af --- /dev/null +++ b/mdl/executor/cmd_module_documentation_test.go @@ -0,0 +1,158 @@ +// SPDX-License-Identifier: Apache-2.0 + +// mendixlabs/mxcli#1314: since #1269 lint reads +// modules().domain_model_documentation and project_security().admin_user_name, +// and MDL could write neither — every MDL-built module had empty documentation +// and the admin was always MxAdmin, so the rules over them had no negative case. +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" + "github.com/mendixlabs/mxcli/sdk/security" +) + +type dmDocCapture struct { + calls []string + dmID model.ID +} + +// moduleDocBackend serves one module, existing or not, whose domain model has +// the given documentation, and records SetDomainModelDocumentation calls. +func moduleDocBackend(existing bool, stored string) (*mock.MockBackend, *dmDocCapture) { + cap := &dmDocCapture{} + mod := mkModule("Sales") + dm := &domainmodel.DomainModel{BaseElement: model.BaseElement{ID: "dm-sales"}, + ContainerID: mod.ID, Documentation: stored} + mods := []*model.Module{} + if existing { + mods = append(mods, mod) + } + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return mods, nil }, + CreateModuleFunc: func(m *model.Module) error { + m.ID = mod.ID + mods = append(mods, m) + return nil + }, + GetDomainModelFunc: func(id model.ID) (*domainmodel.DomainModel, error) { + if id != mod.ID { + return nil, nil + } + return dm, nil + }, + SetDomainModelDocumentationFunc: func(id model.ID, doc string) error { + cap.dmID = id + cap.calls = append(cap.calls, doc) + dm.Documentation = doc + return nil + }, + GetModuleSecurityFunc: func(model.ID) (*security.ModuleSecurity, error) { return nil, nil }, + } + return mb, cap +} + +func TestCreateModule_DocCommentSetsDomainModelDocumentation(t *testing.T) { + for _, tc := range []struct { + name string + existing bool + stmt *ast.CreateModuleStmt + want []string + }{ + {"new module", false, + &ast.CreateModuleStmt{Name: "Sales", Documentation: "Sales data", DocumentationSet: true}, + []string{"Sales data"}}, + {"existing module, or modify", true, + &ast.CreateModuleStmt{Name: "Sales", Documentation: "Sales data", DocumentationSet: true, CreateOrModify: true}, + []string{"Sales data"}}, + {"empty comment clears", true, + &ast.CreateModuleStmt{Name: "Sales", DocumentationSet: true, CreateOrModify: true}, + []string{""}}, + // Controls: a statement that says nothing about documentation, and a + // plain create of a module that exists (a no-op, as it always was), + // must not write it. + {"control: no comment", true, + &ast.CreateModuleStmt{Name: "Sales", CreateOrModify: true}, nil}, + {"control: plain create of an existing module", true, + &ast.CreateModuleStmt{Name: "Sales", Documentation: "Sales data", DocumentationSet: true}, nil}, + } { + t.Run(tc.name, func(t *testing.T) { + mb, cap := moduleDocBackend(tc.existing, "stored by Studio Pro") + ctx, _ := newMockCtx(t, withBackend(mb)) + assertNoError(t, execCreateModule(ctx, tc.stmt)) + if strings.Join(cap.calls, "|") != strings.Join(tc.want, "|") || len(cap.calls) != len(tc.want) { + t.Fatalf("SetDomainModelDocumentation calls = %q, want %q", cap.calls, tc.want) + } + if len(tc.want) > 0 && cap.dmID != "dm-sales" { + t.Errorf("wrote documentation to %q, want the module's domain model", cap.dmID) + } + }) + } +} + +// describe module prints the documentation as the doc comment that sets it, so +// its output re-run stores the same text. +func TestDescribeModule_EmitsDomainModelDocumentation(t *testing.T) { + mb, _ := moduleDocBackend(true, "Sales data,\nper region") + ctx, buf := newMockCtx(t, withBackend(mb)) + assertNoError(t, describeModule(ctx, "Sales", false)) + + prog, errs := visitor.Build(buf.String()) + if len(errs) > 0 { + t.Fatalf("describe module emitted MDL the parser rejects: %v\n%s", errs, buf.String()) + } + var got *ast.CreateModuleStmt + for _, s := range prog.Statements { + if m, ok := s.(*ast.CreateModuleStmt); ok { + got = m + } + } + if got == nil || !got.DocumentationSet || got.Documentation != "Sales data,\nper region" { + t.Fatalf("describe module does not round-trip the documentation: %+v\n%s", got, buf.String()) + } + + // Control: no documentation, no comment. + mb, _ = moduleDocBackend(true, "") + ctx, buf = newMockCtx(t, withBackend(mb)) + assertNoError(t, describeModule(ctx, "Sales", false)) + if strings.Contains(buf.String(), "/**") { + t.Errorf("describe module printed a doc comment for an undocumented domain model:\n%s", buf.String()) + } +} + +func TestAlterProjectSecurity_AdminUserName(t *testing.T) { + var gotID model.ID + var gotName string + called := false + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + GetProjectSecurityFunc: func() (*security.ProjectSecurity, error) { + return &security.ProjectSecurity{BaseElement: model.BaseElement{ID: "ps-1"}, AdminUserName: "MxAdmin"}, nil + }, + SetProjectAdminUserNameFunc: func(id model.ID, name string) error { + called, gotID, gotName = true, id, name + return nil + }, + } + ctx, _ := newMockCtx(t, withBackend(mb)) + assertNoError(t, execAlterProjectSecurity(ctx, &ast.AlterProjectSecurityStmt{AdminUserName: "appadmin"})) + if !called || gotID != "ps-1" || gotName != "appadmin" { + t.Fatalf("SetProjectAdminUserName(%q, %q) called=%v, want (ps-1, appadmin)", gotID, gotName, called) + } + + // Control: a statement about something else leaves the admin name alone. + called = false + on := true + mb.SetProjectStrictModeFunc = func(model.ID, bool) error { return nil } + assertNoError(t, execAlterProjectSecurity(ctx, &ast.AlterProjectSecurityStmt{StrictModeEnabled: &on})) + if called { + t.Error("a StrictMode statement wrote the admin user name") + } +} diff --git a/mdl/executor/cmd_modules.go b/mdl/executor/cmd_modules.go index 199d9646f6..e90663274c 100644 --- a/mdl/executor/cmd_modules.go +++ b/mdl/executor/cmd_modules.go @@ -32,6 +32,16 @@ func execCreateModule(ctx *ExecContext, s *ast.CreateModuleStmt) error { for _, m := range modules { if m.Name == s.Name { + // A plain create of an existing module stays the no-op it always + // was; `or modify` applies the documentation the statement states. + if s.CreateOrModify && s.DocumentationSet { + return setModuleDocumentation(ctx, m, s.Documentation) + } + if s.DocumentationSet { + fmt.Fprintf(ctx.Output, "Module '%s' already exists; documentation unchanged "+ + "(create or modify module sets it)\n", s.Name) + return nil + } fmt.Fprintf(ctx.Output, "Module '%s' already exists\n", s.Name) return nil } @@ -50,6 +60,28 @@ func execCreateModule(ctx *ExecContext, s *ast.CreateModuleStmt) error { invalidateModuleCache(ctx) fmt.Fprintf(ctx.Output, "Created module: %s\n", s.Name) + if s.DocumentationSet && s.Documentation != "" { + return setModuleDocumentation(ctx, module, s.Documentation) + } + return nil +} + +// setModuleDocumentation stores a module's doc comment as its domain model's +// documentation: a Mendix module has no documentation property, its +// DomainModels$DomainModel does, and that is what lint reads as +// modules().domain_model_documentation (mendixlabs/mxcli#1314). +func setModuleDocumentation(ctx *ExecContext, module *model.Module, doc string) error { + dm, err := ctx.Backend.GetDomainModel(module.ID) + if err != nil { + return mdlerrors.NewBackend(fmt.Sprintf("read domain model of %s", module.Name), err) + } + if dm == nil { + return mdlerrors.NewNotFound("domain model of module", module.Name) + } + if err := ctx.Backend.SetDomainModelDocumentation(dm.ID, doc); err != nil { + return mdlerrors.NewBackend(fmt.Sprintf("set documentation of %s", module.Name), err) + } + fmt.Fprintf(ctx.Output, "Set documentation of module: %s\n", module.Name) return nil } @@ -611,7 +643,11 @@ func describeModule(ctx *ExecContext, moduleName string, withAll bool) error { return mdlerrors.NewNotFound("module", moduleName) } - // Output basic CREATE MODULE statement + // The module's documentation is its domain model's; print it as the doc + // comment that sets it, so the output re-run stores it (mendixlabs/mxcli#1314). + if dm, err := ctx.Backend.GetDomainModel(targetModule.ID); err == nil && dm != nil && dm.Documentation != "" { + fmt.Fprintf(ctx.Output, "/**\n * %s\n */\n", strings.ReplaceAll(dm.Documentation, "\n", "\n * ")) + } fmt.Fprintf(ctx.Output, "create or modify module %s;\n", targetModule.Name) // Module roles live in the module's own Security$ModuleSecurity unit rather diff --git a/mdl/executor/cmd_security_write.go b/mdl/executor/cmd_security_write.go index e36e76231b..6765d4ce1e 100644 --- a/mdl/executor/cmd_security_write.go +++ b/mdl/executor/cmd_security_write.go @@ -1226,6 +1226,13 @@ func execAlterProjectSecurity(ctx *ExecContext, s *ast.AlterProjectSecurityStmt) } } + if s.AdminUserName != "" { + if err := ctx.Backend.SetProjectAdminUserName(ps.ID, s.AdminUserName); err != nil { + return mdlerrors.NewBackend("set admin user name", err) + } + ctx.reportWrite("admin user name: "+s.AdminUserName, "Admin user name set to %s", s.AdminUserName) + } + if s.StrictModeEnabled != nil { if err := ctx.Backend.SetProjectStrictMode(ps.ID, *s.StrictModeEnabled); err != nil { return mdlerrors.NewBackend("set strict mode", err) diff --git a/mdl/executor/documentation_coverage_test.go b/mdl/executor/documentation_coverage_test.go index 2043df1b95..e0ff3ecdec 100644 --- a/mdl/executor/documentation_coverage_test.go +++ b/mdl/executor/documentation_coverage_test.go @@ -55,6 +55,7 @@ var docCarryDone = map[string]bool{ "CreateKnowledgeBaseStmt": true, "CreateConsumedMCPServiceStmt": true, "CreateAgentStmt": true, + "CreateModuleStmt": true, } // docCarryPending is empty: every statement type in scope is carried AND diff --git a/mdl/executor/documentation_preserved_test.go b/mdl/executor/documentation_preserved_test.go index d519ed5c62..f1e7193fd8 100644 --- a/mdl/executor/documentation_preserved_test.go +++ b/mdl/executor/documentation_preserved_test.go @@ -274,6 +274,14 @@ func docPreserveCases() []docPreserveCase { rewrite: "create or replace enumeration TestModule.DocEnum ( A 'A', B 'B', C 'C' );", describe: "describe enumeration TestModule.DocEnum", }, + { + // The module's documentation is its domain model's + // (mendixlabs/mxcli#1314). + name: "module", + create: doc + "create module DocModule1314;", + rewrite: "create or modify module DocModule1314;", + describe: "describe module DocModule1314", + }, } } diff --git a/mdl/grammar/domains/MDLDomainModel.g4 b/mdl/grammar/domains/MDLDomainModel.g4 index 657723de89..e2bb75ea75 100644 --- a/mdl/grammar/domains/MDLDomainModel.g4 +++ b/mdl/grammar/domains/MDLDomainModel.g4 @@ -383,9 +383,9 @@ moduleOptions : moduleOption+ ; -// COMMENT is deliberately absent, and unlike the others it could never have -// worked: Projects$Module has no Documentation property, so there is nowhere in -// the model for a module comment to go. +// COMMENT is deliberately absent: Projects$Module has no Documentation +// property. A module's documentation is its domain model's, and is written as +// the statement's `/** … */` doc comment (mendixlabs/mxcli#1314). moduleOption : FOLDER STRING_LITERAL ; diff --git a/mdl/visitor/visitor_doc_comment_placement_test.go b/mdl/visitor/visitor_doc_comment_placement_test.go index 7228207685..c0def0d4a2 100644 --- a/mdl/visitor/visitor_doc_comment_placement_test.go +++ b/mdl/visitor/visitor_doc_comment_placement_test.go @@ -40,8 +40,8 @@ func TestDetachedDocComments(t *testing.T) { src: "/** e */\ncreate persistent entity M.E (Name: String(20));\n/** c */\ncreate constant M.C (Type: String, DefaultValue: 'x');\n", }, { name: "a create that stores no documentation", - src: "/** m */\ncreate module Sales;\n/** e */\ncreate persistent entity Sales.E (Name: String(20));\n", - want: []ast.DetachedDocComment{{Line: 1, Statement: "create module Sales", + src: "/** m */\ncreate module role Sales.User;\n/** e */\ncreate persistent entity Sales.E (Name: String(20));\n", + want: []ast.DetachedDocComment{{Line: 1, Statement: "create module role Sales.User", Next: "create persistent entity Sales.E", NextLine: 4}}, }} { t.Run(tc.name, func(t *testing.T) { diff --git a/mdl/visitor/visitor_module.go b/mdl/visitor/visitor_module.go index 6a45078406..43ad7a99e5 100644 --- a/mdl/visitor/visitor_module.go +++ b/mdl/visitor/visitor_module.go @@ -12,9 +12,14 @@ func (b *Builder) ExitCreateModuleStatement(ctx *parser.CreateModuleStatementCon if iok := ctx.IdentifierOrKeyword(); iok != nil { name = identifierOrKeywordText(iok) } - b.statements = append(b.statements, &ast.CreateModuleStmt{ - Name: name, - }) + stmt := &ast.CreateModuleStmt{Name: name} + if createStmt := findParentCreateStatement(ctx); createStmt != nil { + stmt.CreateOrModify = createStmt.OR() != nil && (createStmt.MODIFY() != nil || createStmt.REPLACE() != nil) + } + // The doc comment is the domain model's documentation, the only + // documentation a module has (mendixlabs/mxcli#1314). + stmt.Documentation, stmt.DocumentationSet = findDocComment(ctx) + b.statements = append(b.statements, stmt) } // ---------------------------------------------------------------------------- diff --git a/mdl/visitor/visitor_module_documentation_test.go b/mdl/visitor/visitor_module_documentation_test.go new file mode 100644 index 0000000000..9eb6113b0f --- /dev/null +++ b/mdl/visitor/visitor_module_documentation_test.go @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "reflect" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// mendixlabs/mxcli#1314: "the doc comment before `create or modify module +// MyFirstModule` is lost … that statement cannot store documentation, so the +// comment is ignored [MDL089]". A module has no documentation of its own, but +// its domain model does (DomainModels$DomainModel.Documentation, what lint reads +// as modules().domain_model_documentation), and the doc comment on `create +// module` is where that is spelled — as describe module prints it. +func TestCreateModuleDocCommentIsStored(t *testing.T) { + for _, tc := range []struct { + name string + src string + want *ast.CreateModuleStmt + }{{ + name: "create or modify with a comment", + src: "/** Sales data */\ncreate or modify module MyFirstModule;", + want: &ast.CreateModuleStmt{Name: "MyFirstModule", Documentation: "Sales data", + DocumentationSet: true, CreateOrModify: true}, + }, { + name: "plain create with a comment", + src: "/** Sales data */ create module Sales;", + want: &ast.CreateModuleStmt{Name: "Sales", Documentation: "Sales data", DocumentationSet: true}, + }, { + // An empty comment is the clearing spelling (mendixlabs/mxcli#1018). + name: "empty comment clears", + src: "/** */ create or modify module Sales;", + want: &ast.CreateModuleStmt{Name: "Sales", DocumentationSet: true, CreateOrModify: true}, + }, { + // Control: no comment says nothing about documentation, so a rerun of + // a script that never mentioned it cannot blank what Studio Pro wrote. + name: "control: no comment leaves it unset", + src: "create or modify module Sales;", + want: &ast.CreateModuleStmt{Name: "Sales", CreateOrModify: true}, + }} { + t.Run(tc.name, func(t *testing.T) { + prog := mustBuild(t, tc.src) + if len(prog.Statements) != 1 || !reflect.DeepEqual(prog.Statements[0], tc.want) { + t.Fatalf("got %#v, want %#v", prog.Statements, tc.want) + } + if len(prog.DetachedDocComments) != 0 { + t.Errorf("doc comment reported as lost (MDL089): %+v", prog.DetachedDocComments) + } + }) + } +} + +// mendixlabs/mxcli#1314: "alter app security has no property "AdminUserName"". +// Security$ProjectSecurity.AdminUserName is the name of the built-in +// administrator account (MxAdmin by default); the password stays out of MDL +// (mendixlabs/mxcli#624). +func TestAlterAppSecurityAdminUserName(t *testing.T) { + prog := mustBuild(t, "alter app security (AdminUserName: 'appadmin');") + want := &ast.AlterProjectSecurityStmt{AdminUserName: "appadmin"} + if len(prog.Statements) != 1 || !reflect.DeepEqual(prog.Statements[0], want) { + t.Fatalf("got %#v, want %#v", prog.Statements, want) + } + for src, msg := range map[string]string{ + "alter app security ( AdminUserName: '' );": "empty", + "alter app security ( AdminUserName: true );": "name", + "alter app security ( AdminUserName: 12 );": "name", + } { + if _, errs := Build(src); len(errs) == 0 { + t.Errorf("%s: accepted, want an error mentioning %q", src, msg) + } + } +} diff --git a/mdl/visitor/visitor_security.go b/mdl/visitor/visitor_security.go index 986c303331..e4387a9e13 100644 --- a/mdl/visitor/visitor_security.go +++ b/mdl/visitor/visitor_security.go @@ -562,8 +562,9 @@ func nextToken(stop antlr.Token, ctx antlr.ParserRuleContext) antlr.Token { } // appSecurityKeys are the properties `alter app security ( … )` takes: the -// Security$ProjectSecurity property names. -var appSecurityKeys = []string{"SecurityLevel", "EnableDemoUsers", "EnableGuestAccess", "GuestUserRole", "StrictMode"} +// Security$ProjectSecurity property names. AdminPassword is deliberately absent +// (mendixlabs/mxcli#624). +var appSecurityKeys = []string{"SecurityLevel", "EnableDemoUsers", "EnableGuestAccess", "GuestUserRole", "StrictMode", "AdminUserName"} // appSecurityProperties reads the property list into stmt. A key or value the // list does not take is an error, never an ignored property. @@ -632,6 +633,17 @@ func (b *Builder) appSecurityProperties(stmt *ast.AlterProjectSecurityStmt, opts continue } stmt.GuestUserRole = settingsValueText(sv) + case "AdminUserName": + if sv.STRING_LITERAL() == nil { + b.addError(fmt.Errorf("line %d: AdminUserName takes the administrator's user name as a string, not %s", line, sv.GetText())) + continue + } + name := settingsValueText(sv) + if strings.TrimSpace(name) == "" { + b.addError(fmt.Errorf("line %d: AdminUserName cannot be empty — the administrator account needs a name", line)) + continue + } + stmt.AdminUserName = name } } }