Merge pull request #258 from hjotha/submit/corruption-fix-and-v1-hash

fix: preserve MPR v1 contents hash and UnitID across DROP+CREATE

Author: ako

mdl-examples/bug-tests/258-drop-create-microflow-unitid-preservation.mdl

@@ -0,0 +1,45 @@
+-- ============================================================================
+-- Bug #258: DROP + CREATE OR MODIFY microflow UnitID preservation
+-- ============================================================================
+--
+-- Symptom (before fix):
+--   A DROP MICROFLOW X followed by CREATE OR MODIFY MICROFLOW X in the same
+--   session produced a *different* UnitID for the new row. Studio Pro treats
+--   Unit rows with a different UnitID as unrelated documents and refuses to
+--   open the project:
+--     ".mpr does not look like a Mendix Studio Pro project file"
+--
+-- After fix:
+--   The executor remembers the dropped UnitID (and ContainerID, AllowedRoles)
+--   and reuses them on the subsequent CREATE so Studio Pro sees an in-place
+--   update. Also exercises v1 ContentsHash persistence — MPR v1 projects need
+--   the hash populated on every write or Studio Pro rejects the file as
+--   corrupt.
+--
+-- Usage:
+--   mxcli exec mdl-examples/bug-tests/258-drop-create-microflow-unitid-preservation.mdl -p app.mpr
+--   Open the project in Studio Pro — it must load without corruption errors
+--   and the microflow MF_Replaced must be visible under BugTest258.
+-- ============================================================================
+
+create module BugTest258;
+
+create microflow BugTest258.MF_Original
+    returns string
+(
+    set $return = 'original'
+);
+
+-- Drop + recreate in the same session. Before the fix, this produced a
+-- dangling Unit row with a fresh UUID and the project became unopenable in
+-- Studio Pro. After the fix, the new microflow reuses the original UnitID.
+drop microflow BugTest258.MF_Original;
+
+create or modify microflow BugTest258.MF_Original
+    returns string
+(
+    set $return = 'replaced'
+);
+
+-- DESCRIBE must round-trip cleanly (no placeholder UUIDs leaking).
+describe microflow BugTest258.MF_Original;

mdl/executor/cmd_microflows_create.go

@@ -57,6 +57,8 @@ func execCreateMicroflow(ctx *ExecContext, s *ast.CreateMicroflowStmt) error {
 	// Check if microflow with same name already exists in this module
 	var existingID model.ID
 	var existingContainerID model.ID
+	var existingAllowedRoles []model.ID
+	preserveAllowedRoles := false
 	existingMicroflows, err := ctx.Backend.ListMicroflows()
 	if err != nil {
 		return mdlerrors.NewBackend("check existing microflows", err)
@@ -68,18 +70,35 @@ func execCreateMicroflow(ctx *ExecContext, s *ast.CreateMicroflowStmt) error {
 			}
 			existingID = existing.ID
 			existingContainerID = existing.ContainerID
+			existingAllowedRoles = cloneRoleIDs(existing.AllowedModuleRoles)
+			preserveAllowedRoles = true
 			break
 		}
 	}
 
 	// For CREATE OR REPLACE/MODIFY, reuse the existing ID to preserve references
+	qualifiedName := s.Name.Module + "." + s.Name.Name
 	microflowID := model.ID(types.GenerateID())
 	if existingID != "" {
 		microflowID = existingID
 		// Keep the original folder unless a new folder is explicitly specified
 		if s.Folder == "" {
 			containerID = existingContainerID
 		}
+	} else if dropped := consumeDroppedMicroflow(ctx, qualifiedName); dropped != nil {
+		// A prior DROP MICROFLOW in the same session removed the unit. Reuse
+		// its original UnitID and (unless a new folder is specified)
+		// ContainerID so that Studio Pro sees the rewrite as an in-place
+		// update rather than a delete+insert pair, which produces
+		// ".mpr does not look like a Mendix Studio Pro project file" errors.
+		microflowID = dropped.ID
+		if s.Folder == "" && dropped.ContainerID != "" {
+			containerID = dropped.ContainerID
+		}
+		// consumeDroppedMicroflow removed the cache entry, so we own this
+		// slice — no need to clone it again.
+		existingAllowedRoles = dropped.AllowedRoles
+		preserveAllowedRoles = true
 	}
 
 	// Build the microflow
@@ -94,6 +113,9 @@ func execCreateMicroflow(ctx *ExecContext, s *ast.CreateMicroflowStmt) error {
 		MarkAsUsed:               false,
 		Excluded:                 s.Excluded,
 	}
+	if preserveAllowedRoles {
+		mf.AllowedModuleRoles = existingAllowedRoles
+	}
 
 	// Build entity resolver function for parameter/return types
 	entityResolver := func(qn ast.QualifiedName) model.ID {

mdl/executor/cmd_microflows_drop.go

@@ -32,11 +32,17 @@ func execDropMicroflow(ctx *ExecContext, s *ast.DropMicroflowStmt) error {
 		modID := h.FindModuleID(mf.ContainerID)
 		modName := h.GetModuleName(modID)
 		if modName == s.Name.Module && mf.Name == s.Name.Name {
+			qualifiedName := s.Name.Module + "." + s.Name.Name
+			// Remember the UnitID and ContainerID *before* deletion so that a
+			// subsequent CREATE OR REPLACE/MODIFY for the same qualified name
+			// can reuse them. This keeps Studio Pro compatible by turning
+			// delete+insert into an in-place update from the file's
+			// perspective — same UnitID, same folder, just new bytes.
+			rememberDroppedMicroflow(ctx, qualifiedName, mf.ID, mf.ContainerID, mf.AllowedModuleRoles)
 			if err := ctx.Backend.DeleteMicroflow(mf.ID); err != nil {
 				return mdlerrors.NewBackend("delete microflow", err)
 			}
 			// Clear executor-level caches so subsequent CREATE sees fresh state
-			qualifiedName := s.Name.Module + "." + s.Name.Name
 			if ctx.Cache != nil && ctx.Cache.createdMicroflows != nil {
 				delete(ctx.Cache.createdMicroflows, qualifiedName)
 			}

mdl/executor/cmd_write_handlers_mock_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/mendixlabs/mxcli/sdk/domainmodel"
 	"github.com/mendixlabs/mxcli/sdk/microflows"
 	"github.com/mendixlabs/mxcli/sdk/pages"
+	"github.com/mendixlabs/mxcli/sdk/security"
 )
 
 func TestExecCreateModule_Mock(t *testing.T) {
@@ -435,3 +436,175 @@ func TestExecDropAssociation_Mock_NotFound(t *testing.T) {
 		Name: ast.QualifiedName{Module: "MyModule", Name: "NonExistent"},
 	}))
 }
+
+// TestDropThenCreatePreservesMicroflowUnitID is a regression test for the
+// MPR corruption bug documented in docs/MXCLI_MPR_CORRUPTION_PROMPT_0015.md.
+//
+// When a script runs `DROP MICROFLOW X; CREATE OR MODIFY MICROFLOW X ...` in
+// the same session, the executor used to delete the Unit row and then insert
+// a new one with a freshly generated UUID. Studio Pro treats the rewritten
+// ContainerID/UnitID pair as an unrelated document and refuses to open the
+// resulting .mpr ("file does not look like a Mendix Studio Pro project").
+//
+// The fix records the UnitID of dropped microflows on the executor cache and
+// reuses it when a subsequent CREATE OR REPLACE/MODIFY targets the same
+// qualified name, so the delete+insert behaves like an in-place update.
+func TestDropThenCreatePreservesMicroflowUnitID(t *testing.T) {
+	mod := mkModule("MyModule")
+	mf := mkMicroflow(mod.ID, "DoSomething")
+	originalID := mf.ID
+	mf.AllowedModuleRoles = []model.ID{"MyModule.Admin", "MyModule.User"}
+
+	h := mkHierarchy(mod)
+	withContainer(h, mf.ContainerID, mod.ID)
+
+	listedMicroflows := []*microflows.Microflow{mf}
+	var createdID model.ID
+	var createdRoles []model.ID
+
+	mb := &mock.MockBackend{
+		IsConnectedFunc: func() bool { return true },
+		ListModulesFunc: func() ([]*model.Module, error) {
+			return []*model.Module{mod}, nil
+		},
+		ListMicroflowsFunc: func() ([]*microflows.Microflow, error) {
+			return listedMicroflows, nil
+		},
+		DeleteMicroflowFunc: func(id model.ID) error {
+			// Simulate real deletion: hide the microflow from subsequent
+			// ListMicroflows calls so CREATE OR MODIFY sees no existing unit
+			// (matching the bug reproduction exactly).
+			listedMicroflows = nil
+			return nil
+		},
+		CreateMicroflowFunc: func(m *microflows.Microflow) error {
+			createdID = m.ID
+			createdRoles = cloneRoleIDs(m.AllowedModuleRoles)
+			return nil
+		},
+		GetModuleSecurityFunc: func(moduleID model.ID) (*security.ModuleSecurity, error) {
+			return &security.ModuleSecurity{
+				BaseElement: model.BaseElement{ID: nextID("ms")},
+				ContainerID: moduleID,
+			}, nil
+		},
+		AddModuleRoleFunc: func(moduleSecurityID model.ID, name, description string) error {
+			return nil
+		},
+		ListDomainModelsFunc: func() ([]*domainmodel.DomainModel, error) {
+			return nil, nil
+		},
+		ListConsumedRestServicesFunc: func() ([]*model.ConsumedRestService, error) {
+			return nil, nil
+		},
+	}
+
+	// Need an Executor so ctx.executor is set (trackCreatedMicroflow uses it).
+	exec := New(&bytesWriter{})
+	ctx := &ExecContext{
+		Context:  t.Context(),
+		Backend:  mb,
+		Output:   exec.output,
+		Format:   FormatTable,
+		executor: exec,
+	}
+	exec.backend = mb
+	withHierarchy(h)(ctx)
+
+	if err := execDropMicroflow(ctx, &ast.DropMicroflowStmt{
+		Name: ast.QualifiedName{Module: "MyModule", Name: "DoSomething"},
+	}); err != nil {
+		t.Fatalf("DROP MICROFLOW failed: %v", err)
+	}
+
+	// The UnitID and ContainerID must have been stashed on the cache before deletion.
+	if ctx.Cache == nil || ctx.Cache.droppedMicroflows == nil ||
+		ctx.Cache.droppedMicroflows["MyModule.DoSomething"] == nil ||
+		ctx.Cache.droppedMicroflows["MyModule.DoSomething"].ID != originalID {
+		t.Fatalf("expected droppedMicroflows[MyModule.DoSomething].ID = %q, got cache=%+v",
+			originalID, ctx.Cache)
+	}
+
+	// CREATE OR MODIFY with the same qualified name must reuse the dropped ID.
+	createStmt := &ast.CreateMicroflowStmt{
+		Name:           ast.QualifiedName{Module: "MyModule", Name: "DoSomething"},
+		CreateOrModify: true,
+		Body:           nil, // empty body is fine for this test
+	}
+	if err := execCreateMicroflow(ctx, createStmt); err != nil {
+		t.Fatalf("CREATE OR MODIFY MICROFLOW failed: %v", err)
+	}
+
+	if createdID != originalID {
+		t.Fatalf("CREATE OR MODIFY must reuse dropped UnitID: got %q, want %q",
+			createdID, originalID)
+	}
+	if len(createdRoles) != 2 || createdRoles[0] != "MyModule.Admin" || createdRoles[1] != "MyModule.User" {
+		t.Fatalf("CREATE OR MODIFY must preserve dropped allowed roles: got %v", createdRoles)
+	}
+
+	// The cache entry must be consumed so repeated CREATEs don't collide.
+	if ctx.Cache != nil && ctx.Cache.droppedMicroflows != nil {
+		if _, stillThere := ctx.Cache.droppedMicroflows["MyModule.DoSomething"]; stillThere {
+			t.Errorf("droppedMicroflows entry should be cleared after reuse")
+		}
+	}
+}
+
+func TestCreateOrModifyMicroflowPreservesAllowedRoles(t *testing.T) {
+	mod := mkModule("MyModule")
+	mf := mkMicroflow(mod.ID, "DoSomething")
+	mf.AllowedModuleRoles = []model.ID{"MyModule.Admin"}
+
+	h := mkHierarchy(mod)
+	withContainer(h, mf.ContainerID, mod.ID)
+
+	var updatedRoles []model.ID
+	mb := &mock.MockBackend{
+		IsConnectedFunc: func() bool { return true },
+		ListModulesFunc: func() ([]*model.Module, error) {
+			return []*model.Module{mod}, nil
+		},
+		ListMicroflowsFunc: func() ([]*microflows.Microflow, error) {
+			return []*microflows.Microflow{mf}, nil
+		},
+		UpdateMicroflowFunc: func(updated *microflows.Microflow) error {
+			updatedRoles = cloneRoleIDs(updated.AllowedModuleRoles)
+			return nil
+		},
+		ListDomainModelsFunc: func() ([]*domainmodel.DomainModel, error) {
+			return nil, nil
+		},
+		ListConsumedRestServicesFunc: func() ([]*model.ConsumedRestService, error) {
+			return nil, nil
+		},
+	}
+
+	exec := New(&bytesWriter{})
+	ctx := &ExecContext{
+		Context:  t.Context(),
+		Backend:  mb,
+		Output:   exec.output,
+		Format:   FormatTable,
+		executor: exec,
+	}
+	exec.backend = mb
+	withHierarchy(h)(ctx)
+
+	if err := execCreateMicroflow(ctx, &ast.CreateMicroflowStmt{
+		Name:           ast.QualifiedName{Module: "MyModule", Name: "DoSomething"},
+		CreateOrModify: true,
+	}); err != nil {
+		t.Fatalf("CREATE OR MODIFY MICROFLOW failed: %v", err)
+	}
+
+	if len(updatedRoles) != 1 || updatedRoles[0] != "MyModule.Admin" {
+		t.Fatalf("expected existing allowed roles to be preserved, got %v", updatedRoles)
+	}
+}
+
+// bytesWriter is a trivial io.Writer used to satisfy New() in the regression
+// test above. We don't care about captured output for this test.
+type bytesWriter struct{}
+
+func (*bytesWriter) Write(p []byte) (int, error) { return len(p), nil }

mdl/executor/executor.go

@@ -35,6 +35,15 @@ type executorCache struct {
 	createdPages      map[string]*createdPageInfo      // qualifiedName -> info
 	createdSnippets   map[string]*createdSnippetInfo   // qualifiedName -> info
 
+	// Track items dropped during this session so that a subsequent
+	// CREATE OR REPLACE/MODIFY with the same qualified name can reuse the
+	// original UnitID and ContainerID. Studio Pro treats Unit rows with a
+	// different UnitID (or the same UnitID under a different container) as
+	// unrelated documents, producing broken projects on delete+insert
+	// rewrites. Reusing both keeps the rewrite semantically equivalent to an
+	// in-place update.
+	droppedMicroflows map[string]*droppedUnitInfo // qualifiedName -> original IDs
+
 	// Track domain models modified during this session for finalization
 	modifiedDomainModels map[model.ID]string // domain model unit ID -> module name
 
@@ -69,6 +78,15 @@ type createdSnippetInfo struct {
 	ContainerID model.ID
 }
 
+// droppedUnitInfo remembers the original UnitID and ContainerID of a document
+// dropped during this session so that a subsequent CREATE OR REPLACE/MODIFY
+// with the same qualified name can reuse them instead of generating new UUIDs.
+type droppedUnitInfo struct {
+	ID           model.ID
+	ContainerID  model.ID
+	AllowedRoles []model.ID
+}
+
 // getEntityNames returns the entity name lookup map, using the pre-warmed cache if available.
 func getEntityNames(ctx *ExecContext, h *ContainerHierarchy) map[model.ID]string {
 	if ctx.Cache != nil && len(ctx.Cache.entityNames) > 0 {
@@ -416,3 +434,53 @@ func (e *Executor) ensureCache() {
 		e.cache = &executorCache{}
 	}
 }
+
+// rememberDroppedMicroflow records the UnitID and ContainerID of a microflow
+// that is about to be deleted via DROP MICROFLOW. A follow-up CREATE OR
+// REPLACE/MODIFY for the same qualified name will reuse both instead of
+// generating a fresh UUID and defaulting to the module root, so Studio Pro
+// continues to see the unit as "updated in place" rather than a delete+insert
+// pair.
+func rememberDroppedMicroflow(ctx *ExecContext, qualifiedName string, id, containerID model.ID, allowedRoles []model.ID) {
+	if ctx == nil || qualifiedName == "" || id == "" {
+		return
+	}
+	if ctx.Cache == nil {
+		ctx.Cache = &executorCache{}
+		if ctx.executor != nil {
+			ctx.executor.cache = ctx.Cache
+		}
+	}
+	if ctx.Cache.droppedMicroflows == nil {
+		ctx.Cache.droppedMicroflows = make(map[string]*droppedUnitInfo)
+	}
+	ctx.Cache.droppedMicroflows[qualifiedName] = &droppedUnitInfo{
+		ID:           id,
+		ContainerID:  containerID,
+		AllowedRoles: cloneRoleIDs(allowedRoles),
+	}
+}
+
+func cloneRoleIDs(roles []model.ID) []model.ID {
+	if len(roles) == 0 {
+		return nil
+	}
+	cloned := make([]model.ID, len(roles))
+	copy(cloned, roles)
+	return cloned
+}
+
+// consumeDroppedMicroflow returns the original IDs of a microflow dropped
+// earlier in this session (if any) and removes the entry so repeated CREATEs
+// don't collide on the same ID. Returns nil when nothing was remembered.
+func consumeDroppedMicroflow(ctx *ExecContext, qualifiedName string) *droppedUnitInfo {
+	if ctx == nil || ctx.Cache == nil || ctx.Cache.droppedMicroflows == nil {
+		return nil
+	}
+	info, ok := ctx.Cache.droppedMicroflows[qualifiedName]
+	if !ok {
+		return nil
+	}
+	delete(ctx.Cache.droppedMicroflows, qualifiedName)
+	return info
+}