Skip to content

Commit 5d8c464

Browse files
hjothahjothamendix
authored andcommitted
fix: stabilize MDL roundtrip and integration flows
Four independent stability fixes surfaced together while chasing a green roundtrip suite. They touch different layers but are bundled here because they all gate on the same test run — splitting would require duplicated test fixtures across PRs. sdk/mpr — FindOrCreate mapping roundtrip: Import/export mappings store "FindOrCreate" in BSON as a *pair* of fields: `ObjectHandling=Find` plus `ObjectHandlingBackup=Create`. The parser previously copied `ObjectHandling` verbatim (yielding just "Find"), and the export writer hardcoded `ObjectHandlingBackup=Create` regardless of the primary mode. - `parser_import_mapping.go` now recognises the `Find + Create` pair and promotes it to the synthetic `FindOrCreate` value the domain model uses elsewhere. - `writer_export_mapping.go` echoes the actual `ObjectHandling` instead of forcing "Create"; misuse produced CE6702 on describe + re-execute cycles. - `writer_import_mapping.go` inverts the split symmetrically: if the model says `FindOrCreate`, serialise as `Find` + `Create` so Studio Pro can load the mapping. Added parser/writer tests cover both halves and a matching fixture (`RestTest.PetRecord`) was added to `06-rest-client-examples.mdl` so the mapping has a real entity to bind to; this lets `roundtrip_doctype_test.go` drop CE6702 from the known-errors list. mdl/executor — DROP + CREATE OR REPLACE preserves UnitID: When a script ran `DROP MICROFLOW X; CREATE OR MODIFY MICROFLOW X ...` in the same session, the executor deleted the Unit row and inserted a new one with a fresh UUID. Studio Pro treats the new UnitID as an unrelated document and rejects the file with ".mpr does not look like a Mendix Studio Pro project". - `executor.go` adds a `droppedMicroflows` cache on `executorCache` keyed by qualified name, plus `rememberDroppedMicroflow` / `consumeDroppedMicroflow` helpers. - `cmd_microflows_drop.go` records the UnitID, ContainerID, and AllowedModuleRoles before calling `DeleteMicroflow`. - `cmd_microflows_create.go` consumes the remembered entry when the qualified name matches, reusing the original UnitID (and ContainerID when no new folder is given) so the rewrite looks like an in-place update from the file's perspective. - Mock regression test `TestDropThenCreatePreservesMicroflowUnitID` asserts the reused UnitID and preserved access roles. mdl/executor — default document access roles: Fresh `CREATE MICROFLOW` / `CREATE PAGE` on a module with no module roles tripped CE0148 ("document has no access for any role"). Rather than leaving every script author to define roles by hand, auto- provision a minimal `User` role on modules that currently have none, and stamp it onto new pages/microflows. - `cmd_security_defaults.go` (new) adds `defaultDocumentAccessRoles`, `remapDocumentAccessRoles` (cross-module MOVE), `documentRoleStrings`, `cloneRoleIDs`, and `pruneInvalidUserRoles`. The auto-created role carries a fixed description (`autoDocumentRoleDescription`) so `CREATE MODULE ROLE User` on the same module is treated as a no-op rather than CE0112 "already exists". - `cmd_microflows_create.go` and `cmd_pages_create_v3.go` stamp the default on new units and preserve existing roles across `OR REPLACE` / `OR MODIFY`. - `cmd_move.go` remaps access roles across cross-module moves using the target module's role names (falling back to the default role when the target has none). - `cmd_modules.go` and `cmd_security_write.go` call `pruneInvalidUserRoles` after DROP MODULE / DROP MODULE ROLE so orphaned user roles don't trip CE0157. - `cmd_microflows_builder.go` — `registerResultVariableType(nil)` now clears stale entity typing and marks the variable as declared with type `Unknown`, instead of leaving the variable un-tracked, so subsequent CHANGE/attribute access still resolves. Covered by `TestCallMicroflowUnknownResultTypeStillDeclaresVariable`. Test and script hardening: - `docker/check_test.go` and `docker/detect_test.go` set `PATH=t.TempDir()` instead of `PATH=""` for the cache-preference tests. Empty `PATH` confuses `exec.LookPath` on Linux (it falls back to the current directory) and made the test flaky depending on `os.Getwd()`. An empty temp directory is a clean "mx isn't on PATH" signal. - `scripts/run-mdl-tests.sh` now validates `$PROJECT_MPR`, `$MXCLI_BIN`, `$TEST_SPEC`, `$BOOTSTRAP_MDL` with `[[ -f ... ]]` guards. `${VAR:?...}` only fires on *unset* vars, so typo'd paths previously produced an empty sandbox and confusing downstream errors.
1 parent afd96c3 commit 5d8c464

19 files changed

Lines changed: 307 additions & 20 deletions

cmd/mxcli/docker/check_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,9 @@ func TestCheck_SkipUpdateWidgetsFlag(t *testing.T) {
122122
func TestResolveMxForVersion_PrefersExactCachedVersion(t *testing.T) {
123123
dir := t.TempDir()
124124
setTestHomeDir(t, dir)
125-
t.Setenv("PATH", "")
125+
// Point PATH at an empty temp dir (rather than clearing it) so exec.LookPath
126+
// still works for any other testing infrastructure but can't find mx.
127+
t.Setenv("PATH", t.TempDir())
126128

127129
versions := []string{"9.24.40.80973", "11.6.3", "11.9.0"}
128130
var expected string

cmd/mxcli/docker/detect_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,9 @@ func TestResolveMxBuild_PrefersStudioProOverCache(t *testing.T) {
272272
func TestResolveMxBuild_PrefersExactCachedVersion(t *testing.T) {
273273
dir := t.TempDir()
274274
setTestHomeDir(t, dir)
275-
t.Setenv("PATH", "")
275+
// Point PATH at an empty temp dir (rather than clearing it) so exec.LookPath
276+
// still works for any other testing infrastructure but can't find mxbuild.
277+
t.Setenv("PATH", t.TempDir())
276278

277279
versions := []string{"9.24.40.80973", "11.6.3", "11.9.0"}
278280
var expected string

mdl-examples/doctype-tests/06-rest-client-examples.mdl

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1301,10 +1301,17 @@ create import mapping RestTest.IMM_Order
13011301
--
13021302
-- Uses FIND OR CREATE to upsert: find existing by KEY, create if not found.
13031303

1304+
create persistent entity RestTest.PetRecord (
1305+
PetId: Integer,
1306+
Name: String,
1307+
Status: String
1308+
);
1309+
/
1310+
13041311
create import mapping RestTest.IMM_UpsertPet
13051312
with json structure RestTest.JSON_Pet
13061313
{
1307-
find or create RestTest.PetResponse {
1314+
find or create RestTest.PetRecord {
13081315
PetId = id key,
13091316
Name = name,
13101317
status = status

mdl/executor/bugfix_regression_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,3 +509,25 @@ func TestCallMicroflowResultType_ResolvesSubsequentChangeMember(t *testing.T) {
509509
t.Fatalf("expected qualified attribute MfTest.Product.Price, got %q", got)
510510
}
511511
}
512+
513+
func TestCallMicroflowUnknownResultTypeStillDeclaresVariable(t *testing.T) {
514+
fb := &flowBuilder{
515+
varTypes: map[string]string{"Result": "Old.ModuleEntity"},
516+
declaredVars: map[string]string{},
517+
}
518+
519+
fb.addCallMicroflowAction(&ast.CallMicroflowStmt{
520+
OutputVariable: "Result",
521+
MicroflowName: ast.QualifiedName{Module: "Missing", Name: "Unknown"},
522+
})
523+
524+
if _, ok := fb.varTypes["Result"]; ok {
525+
t.Fatalf("expected stale entity typing to be cleared, got %q", fb.varTypes["Result"])
526+
}
527+
if got := fb.declaredVars["Result"]; got != "Unknown" {
528+
t.Fatalf("expected Result to remain declared as Unknown, got %q", got)
529+
}
530+
if !fb.isVariableDeclared("Result") {
531+
t.Fatal("expected Result to remain declared after unresolved call return type")
532+
}
533+
}

mdl/executor/cmd_microflows_builder.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,20 @@ func (fb *flowBuilder) isVariableDeclared(varName string) bool {
7979

8080
// registerResultVariableType records the output type of an action so later
8181
// statements such as CHANGE, ADD TO, or attribute access can resolve members.
82+
// When dt is nil (e.g. backend lookup failed), any stale entity/list typing is
83+
// cleared but the variable remains declared as Unknown so downstream statements
84+
// don't report it as undeclared.
8285
func (fb *flowBuilder) registerResultVariableType(varName string, dt microflows.DataType) {
83-
if varName == "" || dt == nil {
86+
if varName == "" {
87+
return
88+
}
89+
if dt == nil {
90+
if fb.varTypes != nil {
91+
delete(fb.varTypes, varName)
92+
}
93+
if fb.declaredVars != nil {
94+
fb.declaredVars[varName] = "Unknown"
95+
}
8496
return
8597
}
8698

mdl/executor/cmd_microflows_create.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@ func execCreateMicroflow(ctx *ExecContext, s *ast.CreateMicroflowStmt) error {
115115
}
116116
if preserveAllowedRoles {
117117
mf.AllowedModuleRoles = existingAllowedRoles
118+
} else {
119+
mf.AllowedModuleRoles = defaultDocumentAccessRoles(ctx, module)
118120
}
119121

120122
// Build entity resolver function for parameter/return types

mdl/executor/cmd_modules.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,9 @@ func execDropModule(ctx *ExecContext, s *ast.DropModuleStmt) error {
276276
fmt.Fprintf(ctx.Output, "Removed %s from %d user role(s)\n", qualifiedRole, n)
277277
}
278278
}
279+
if err := pruneInvalidUserRoles(ctx, ps); err != nil {
280+
return mdlerrors.NewBackend("cleanup invalid user roles", err)
281+
}
279282
}
280283
}
281284

mdl/executor/cmd_move.go

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,11 @@ func execMove(ctx *ExecContext, s *ast.MoveStmt) error {
5656
// Execute move based on document type
5757
switch s.DocumentType {
5858
case ast.DocumentTypePage:
59-
if err := movePage(ctx, s.Name, targetContainerID); err != nil {
59+
if err := movePage(ctx, s.Name, targetContainerID, targetModule, isCrossModuleMove); err != nil {
6060
return err
6161
}
6262
case ast.DocumentTypeMicroflow:
63-
if err := moveMicroflow(ctx, s.Name, targetContainerID); err != nil {
63+
if err := moveMicroflow(ctx, s.Name, targetContainerID, targetModule, isCrossModuleMove); err != nil {
6464
return err
6565
}
6666
case ast.DocumentTypeSnippet:
@@ -110,7 +110,7 @@ func updateQualifiedNameRefs(ctx *ExecContext, name ast.QualifiedName, newModule
110110
}
111111

112112
// movePage moves a page to a new container.
113-
func movePage(ctx *ExecContext, name ast.QualifiedName, targetContainerID model.ID) error {
113+
func movePage(ctx *ExecContext, name ast.QualifiedName, targetContainerID model.ID, targetModule *model.Module, isCrossModuleMove bool) error {
114114
// Find the page
115115
pages, err := ctx.Backend.ListPages()
116116
if err != nil {
@@ -131,6 +131,12 @@ func movePage(ctx *ExecContext, name ast.QualifiedName, targetContainerID model.
131131
if err := ctx.Backend.MovePage(p); err != nil {
132132
return mdlerrors.NewBackend("move page", err)
133133
}
134+
if isCrossModuleMove {
135+
p.AllowedRoles = remapDocumentAccessRoles(ctx, targetModule, p.AllowedRoles)
136+
if err := ctx.Backend.UpdateAllowedRoles(p.ID, documentRoleStrings(p.AllowedRoles)); err != nil {
137+
return mdlerrors.NewBackend("remap page access", err)
138+
}
139+
}
134140
fmt.Fprintf(ctx.Output, "Moved page %s to new location\n", name.String())
135141
return nil
136142
}
@@ -140,7 +146,7 @@ func movePage(ctx *ExecContext, name ast.QualifiedName, targetContainerID model.
140146
}
141147

142148
// moveMicroflow moves a microflow to a new container.
143-
func moveMicroflow(ctx *ExecContext, name ast.QualifiedName, targetContainerID model.ID) error {
149+
func moveMicroflow(ctx *ExecContext, name ast.QualifiedName, targetContainerID model.ID, targetModule *model.Module, isCrossModuleMove bool) error {
144150
// Find the microflow
145151
mfs, err := ctx.Backend.ListMicroflows()
146152
if err != nil {
@@ -161,6 +167,12 @@ func moveMicroflow(ctx *ExecContext, name ast.QualifiedName, targetContainerID m
161167
if err := ctx.Backend.MoveMicroflow(mf); err != nil {
162168
return mdlerrors.NewBackend("move microflow", err)
163169
}
170+
if isCrossModuleMove {
171+
mf.AllowedModuleRoles = remapDocumentAccessRoles(ctx, targetModule, mf.AllowedModuleRoles)
172+
if err := ctx.Backend.UpdateAllowedRoles(mf.ID, documentRoleStrings(mf.AllowedModuleRoles)); err != nil {
173+
return mdlerrors.NewBackend("remap microflow access", err)
174+
}
175+
}
164176
fmt.Fprintf(ctx.Output, "Moved microflow %s to new location\n", name.String())
165177
return nil
166178
}

mdl/executor/cmd_pages_create_v3.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,19 @@ func execCreatePageV3(ctx *ExecContext, s *ast.CreatePageStmtV3) error {
4040
// Check if page already exists - collect ALL duplicates
4141
existingPages, _ := ctx.Backend.ListPages()
4242
var pagesToDelete []model.ID
43+
var existingAllowedRoles []model.ID
44+
preserveAllowedRoles := false
4345
for _, p := range existingPages {
4446
modID := getModuleID(ctx, p.ContainerID)
4547
modName := getModuleName(ctx, modID)
4648
if modName == s.Name.Module && p.Name == s.Name.Name {
4749
if !s.IsReplace && !s.IsModify && len(pagesToDelete) == 0 {
4850
return mdlerrors.NewAlreadyExists("page", s.Name.String())
4951
}
52+
if len(pagesToDelete) == 0 {
53+
existingAllowedRoles = cloneRoleIDs(p.AllowedRoles)
54+
preserveAllowedRoles = true
55+
}
5056
pagesToDelete = append(pagesToDelete, p.ID)
5157
}
5258
}
@@ -69,6 +75,11 @@ func execCreatePageV3(ctx *ExecContext, s *ast.CreatePageStmtV3) error {
6975
if err != nil {
7076
return mdlerrors.NewBackend("build page", err)
7177
}
78+
if preserveAllowedRoles {
79+
page.AllowedRoles = existingAllowedRoles
80+
} else if len(page.AllowedRoles) == 0 {
81+
page.AllowedRoles = defaultDocumentAccessRoles(ctx, module)
82+
}
7283

7384
// Replace or create the page in the MPR
7485
if len(pagesToDelete) > 0 {
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
3+
package executor
4+
5+
import (
6+
"fmt"
7+
"strings"
8+
9+
"github.com/mendixlabs/mxcli/model"
10+
"github.com/mendixlabs/mxcli/sdk/security"
11+
)
12+
13+
const (
14+
autoDocumentRoleName = "User"
15+
autoDocumentRoleDescription = "Auto-created default role for mxcli document access"
16+
)
17+
18+
// defaultDocumentAccessRoles returns a conservative fallback role set for newly
19+
// created pages/microflows when the target module has no module roles at all.
20+
//
21+
// Mendix accepts document access only when it references a role from the same
22+
// module; using an existing role from another module causes CE0148 on freshly
23+
// created documents. To keep mx-check green, auto-create a local `User` module
24+
// role only for modules that currently have zero roles. Modules that already
25+
// manage their own roles keep the existing "no access by default" behavior.
26+
func defaultDocumentAccessRoles(ctx *ExecContext, module *model.Module) []model.ID {
27+
if module == nil {
28+
return nil
29+
}
30+
31+
ms, err := ctx.Backend.GetModuleSecurity(module.ID)
32+
if err != nil || ms == nil {
33+
return nil
34+
}
35+
if moduleUsesAutoDocumentRole(ms) {
36+
return []model.ID{model.ID(module.Name + "." + autoDocumentRoleName)}
37+
}
38+
if len(ms.ModuleRoles) > 0 {
39+
return nil
40+
}
41+
42+
if err := ctx.Backend.AddModuleRole(ms.ID, autoDocumentRoleName, autoDocumentRoleDescription); err != nil {
43+
return nil
44+
}
45+
return []model.ID{model.ID(module.Name + "." + autoDocumentRoleName)}
46+
}
47+
48+
func moduleUsesAutoDocumentRole(ms *security.ModuleSecurity) bool {
49+
if ms == nil {
50+
return false
51+
}
52+
return len(ms.ModuleRoles) == 1 &&
53+
ms.ModuleRoles[0].Name == autoDocumentRoleName &&
54+
ms.ModuleRoles[0].Description == autoDocumentRoleDescription
55+
}
56+
57+
func remapDocumentAccessRoles(ctx *ExecContext, targetModule *model.Module, currentRoles []model.ID) []model.ID {
58+
if targetModule == nil {
59+
return nil
60+
}
61+
62+
ms, err := ctx.Backend.GetModuleSecurity(targetModule.ID)
63+
if err != nil || ms == nil {
64+
return nil
65+
}
66+
if len(ms.ModuleRoles) == 0 || moduleUsesAutoDocumentRole(ms) {
67+
return defaultDocumentAccessRoles(ctx, targetModule)
68+
}
69+
70+
targetRoleNames := make(map[string]bool, len(ms.ModuleRoles))
71+
for _, role := range ms.ModuleRoles {
72+
targetRoleNames[role.Name] = true
73+
}
74+
75+
var remapped []model.ID
76+
seen := make(map[string]bool)
77+
for _, qualifiedRole := range currentRoles {
78+
roleName := string(qualifiedRole)
79+
if idx := strings.LastIndex(roleName, "."); idx >= 0 {
80+
roleName = roleName[idx+1:]
81+
}
82+
if !targetRoleNames[roleName] {
83+
continue
84+
}
85+
targetQualifiedRole := targetModule.Name + "." + roleName
86+
if seen[targetQualifiedRole] {
87+
continue
88+
}
89+
seen[targetQualifiedRole] = true
90+
remapped = append(remapped, model.ID(targetQualifiedRole))
91+
}
92+
93+
return remapped
94+
}
95+
96+
func documentRoleStrings(roles []model.ID) []string {
97+
values := make([]string, 0, len(roles))
98+
for _, role := range roles {
99+
values = append(values, string(role))
100+
}
101+
return values
102+
}
103+
104+
func cloneRoleIDs(roles []model.ID) []model.ID {
105+
if len(roles) == 0 {
106+
return nil
107+
}
108+
cloned := make([]model.ID, len(roles))
109+
copy(cloned, roles)
110+
return cloned
111+
}
112+
113+
// pruneInvalidUserRoles removes user roles that no longer have any non-System
114+
// module role assignments. Mendix rejects those roles with CE0157.
115+
func pruneInvalidUserRoles(ctx *ExecContext, ps *security.ProjectSecurity) error {
116+
if latest, err := ctx.Backend.GetProjectSecurity(); err == nil {
117+
ps = latest
118+
} else if ps == nil {
119+
return err
120+
}
121+
122+
for _, userRole := range ps.UserRoles {
123+
hasNonSystemRole := false
124+
for _, moduleRole := range userRole.ModuleRoles {
125+
if !strings.HasPrefix(moduleRole, "System.") {
126+
hasNonSystemRole = true
127+
break
128+
}
129+
}
130+
if hasNonSystemRole {
131+
continue
132+
}
133+
if err := ctx.Backend.RemoveUserRole(ps.ID, userRole.Name); err != nil {
134+
return err
135+
}
136+
if !ctx.Quiet {
137+
fmt.Fprintf(ctx.Output, "Dropped invalid user role: %s\n", userRole.Name)
138+
}
139+
}
140+
141+
return nil
142+
}

0 commit comments

Comments
 (0)