diff --git a/mdl/backend/mpr/backend.go b/mdl/backend/mpr/backend.go index da20193b..48e5426b 100644 --- a/mdl/backend/mpr/backend.go +++ b/mdl/backend/mpr/backend.go @@ -85,9 +85,11 @@ func (b *MprBackend) Path() string { return b.path } // for new code. func (b *MprBackend) MprReader() *mpr.Reader { return b.reader } -func (b *MprBackend) Version() types.MPRVersion { return convertMPRVersion(b.reader.Version()) } -func (b *MprBackend) ProjectVersion() *types.ProjectVersion { return convertProjectVersion(b.reader.ProjectVersion()) } -func (b *MprBackend) GetMendixVersion() (string, error) { return b.reader.GetMendixVersion() } +func (b *MprBackend) Version() types.MPRVersion { return convertMPRVersion(b.reader.Version()) } +func (b *MprBackend) ProjectVersion() *types.ProjectVersion { + return convertProjectVersion(b.reader.ProjectVersion()) +} +func (b *MprBackend) GetMendixVersion() (string, error) { return b.reader.GetMendixVersion() } // Commit is a no-op — the MPR writer auto-commits on each write operation. func (b *MprBackend) Commit() error { return nil } @@ -112,7 +114,9 @@ func (b *MprBackend) DeleteModuleWithCleanup(id model.ID, moduleName string) err // FolderBackend // --------------------------------------------------------------------------- -func (b *MprBackend) ListFolders() ([]*types.FolderInfo, error) { return convertFolderInfoSlice(b.reader.ListFolders()) } +func (b *MprBackend) ListFolders() ([]*types.FolderInfo, error) { + return convertFolderInfoSlice(b.reader.ListFolders()) +} func (b *MprBackend) CreateFolder(folder *model.Folder) error { return b.writer.CreateFolder(folder) } func (b *MprBackend) DeleteFolder(id model.ID) error { return b.writer.DeleteFolder(id) } func (b *MprBackend) MoveFolder(id model.ID, newContainerID model.ID) error { @@ -678,8 +682,10 @@ func (b *MprBackend) UpdateRawUnit(unitID string, contents []byte) error { // MetadataBackend // --------------------------------------------------------------------------- -func (b *MprBackend) ListAllUnitIDs() ([]string, error) { return b.reader.ListAllUnitIDs() } -func (b *MprBackend) ListUnits() ([]*types.UnitInfo, error) { return convertUnitInfoSlice(b.reader.ListUnits()) } +func (b *MprBackend) ListAllUnitIDs() ([]string, error) { return b.reader.ListAllUnitIDs() } +func (b *MprBackend) ListUnits() ([]*types.UnitInfo, error) { + return convertUnitInfoSlice(b.reader.ListUnits()) +} func (b *MprBackend) GetUnitTypes() (map[string]int, error) { return b.reader.GetUnitTypes() } func (b *MprBackend) GetProjectRootID() (string, error) { return b.reader.GetProjectRootID() } func (b *MprBackend) ContentsDir() string { return b.reader.ContentsDir() } diff --git a/mdl/backend/mpr/convert_roundtrip_test.go b/mdl/backend/mpr/convert_roundtrip_test.go index a10391fc..157572f0 100644 --- a/mdl/backend/mpr/convert_roundtrip_test.go +++ b/mdl/backend/mpr/convert_roundtrip_test.go @@ -648,4 +648,3 @@ func TestFieldCountDrift(t *testing.T) { assertFieldCount(t, "mpr.EntityAccessRevocation", mpr.EntityAccessRevocation{}, 6) assertFieldCount(t, "types.EntityAccessRevocation", types.EntityAccessRevocation{}, 6) } - diff --git a/mdl/backend/mpr/workflow_mutator.go b/mdl/backend/mpr/workflow_mutator.go index 4e7b86e2..24af21cb 100644 --- a/mdl/backend/mpr/workflow_mutator.go +++ b/mdl/backend/mpr/workflow_mutator.go @@ -162,14 +162,19 @@ func (m *mprWorkflowMutator) SetActivityProperty(activityRef string, atPos int, case "page": taskPage := dGetDoc(actDoc, "TaskPage") if taskPage != nil { + // TaskPage exists and has a value — update the Page field in place. dSet(taskPage, "Page", value) - } else { - pageRef := bson.D{ - {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, - {Key: "$Type", Value: "Workflows$PageReference"}, - {Key: "Page", Value: value}, - } - dSet(actDoc, "TaskPage", pageRef) + return nil + } + pageRef := bson.D{ + {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, + {Key: "$Type", Value: "Workflows$PageReference"}, + {Key: "Page", Value: value}, + } + if !dSet(actDoc, "TaskPage", pageRef) { + // TaskPage key absent — append to activity and replace in BSON tree. + actDoc = append(actDoc, bson.E{Key: "TaskPage", Value: pageRef}) + m.replaceActivity(actDoc) } return nil @@ -555,6 +560,42 @@ func (m *mprWorkflowMutator) Save() error { // Internal helpers — activity search // --------------------------------------------------------------------------- +// replaceActivity replaces an activity document in the workflow's BSON tree +// by matching on $ID. This is needed when appending new keys to an activity +// document, because the slice header returned by findActivityByCaption cannot +// propagate appends back to the parent bson.A. +func (m *mprWorkflowMutator) replaceActivity(updated bson.D) { + actID := extractBinaryIDFromDoc(dGet(updated, "$ID")) + if actID == "" { + return + } + flow := dGetDoc(m.rawData, "Flow") + if flow == nil { + return + } + replaceActivityRecursive(flow, actID, updated) +} + +func replaceActivityRecursive(flow bson.D, actID string, updated bson.D) bool { + elements := dGetArrayElements(dGet(flow, "Activities")) + for i, elem := range elements { + actDoc, ok := elem.(bson.D) + if !ok { + continue + } + if extractBinaryIDFromDoc(dGet(actDoc, "$ID")) == actID { + elements[i] = updated + return true + } + for _, nestedFlow := range getNestedFlows(actDoc) { + if replaceActivityRecursive(nestedFlow, actID, updated) { + return true + } + } + } + return false +} + // findActivityByCaption searches the workflow for an activity matching caption. func (m *mprWorkflowMutator) findActivityByCaption(caption string, atPosition int) (bson.D, error) { flow := dGetDoc(m.rawData, "Flow") diff --git a/mdl/backend/mpr/workflow_mutator_test.go b/mdl/backend/mpr/workflow_mutator_test.go index 09a50a09..1db9f7d1 100644 --- a/mdl/backend/mpr/workflow_mutator_test.go +++ b/mdl/backend/mpr/workflow_mutator_test.go @@ -1136,8 +1136,7 @@ func TestWorkflowMutator_InsertBoundaryEvent_NoDelay(t *testing.T) { // --------------------------------------------------------------------------- func TestWorkflowMutator_SetActivityProperty_Page_New(t *testing.T) { - // Note: When TaskPage key doesn't pre-exist in BSON, dSet silently fails. - // The key must be present (even as nil) for PAGE to work on a new activity. + // TaskPage key present with nil value — should be replaced with a new PageReference. act := makeWfActivity("Workflows$UserTask", "Review", "task1") act = append(act, bson.E{Key: "TaskPage", Value: nil}) m := newMutator(makeWorkflowDoc(act)) @@ -1157,7 +1156,8 @@ func TestWorkflowMutator_SetActivityProperty_Page_New(t *testing.T) { } func TestWorkflowMutator_SetActivityProperty_Page_MissingKey(t *testing.T) { - // BUG: dSet silently fails when TaskPage key is absent — pageRef is lost. + // Regression test: dSet silently failed when TaskPage key was absent. + // Fixed by appending the key to the activity and replacing it in the BSON tree. act := makeWfActivity("Workflows$UserTask", "Review", "task1") // No TaskPage field at all m := newMutator(makeWorkflowDoc(act)) @@ -1169,9 +1169,67 @@ func TestWorkflowMutator_SetActivityProperty_Page_MissingKey(t *testing.T) { actDoc, _ := m.findActivityByCaption("Review", 0) taskPage := dGetDoc(actDoc, "TaskPage") - // This documents the bug: TaskPage is nil because dSet can't create new keys - if taskPage != nil { - t.Log("BUG FIXED: TaskPage is now set even when key was absent") + if taskPage == nil { + t.Fatal("TaskPage should be set even when key was absent") + } + if got := dGetString(taskPage, "Page"); got != "MyModule.TaskPage" { + t.Errorf("Page = %q, want MyModule.TaskPage", got) + } +} + +func TestWorkflowMutator_SetActivityProperty_Page_MissingKey_NestedSubFlow(t *testing.T) { + // Exercises the recursive replaceActivity path: the target activity lives + // inside an outcome's sub-flow, not at the top level. + // Use distinct $IDs so replaceActivity cannot accidentally match the parent. + parentID := primitive.Binary{Subtype: 0x04, Data: []byte{1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}} + nestedID := primitive.Binary{Subtype: 0x04, Data: []byte{2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}} + + nestedAct := bson.D{ + {Key: "$ID", Value: nestedID}, + {Key: "$Type", Value: "Workflows$UserTask"}, + {Key: "Caption", Value: "NestedReview"}, + {Key: "Name", Value: "nested1"}, + } + // No TaskPage field at all on the nested activity. + + outcome := bson.D{ + {Key: "$ID", Value: primitive.Binary{Subtype: 0x04, Data: make([]byte, 16)}}, + {Key: "$Type", Value: "Workflows$BooleanOutcome"}, + {Key: "Flow", Value: bson.D{ + {Key: "$ID", Value: primitive.Binary{Subtype: 0x04, Data: make([]byte, 16)}}, + {Key: "$Type", Value: "Workflows$Flow"}, + {Key: "Activities", Value: bson.A{int32(3), nestedAct}}, + }}, + } + parentAct := bson.D{ + {Key: "$ID", Value: parentID}, + {Key: "$Type", Value: "Workflows$Decision"}, + {Key: "Caption", Value: "Check"}, + {Key: "Name", Value: "decision1"}, + {Key: "Outcomes", Value: bson.A{int32(3), outcome}}, + } + m := newMutator(makeWorkflowDoc(parentAct)) + + if err := m.SetActivityProperty("NestedReview", 0, "PAGE", "MyModule.NestedPage"); err != nil { + t.Fatalf("SetActivityProperty PAGE on nested activity failed: %v", err) + } + + actDoc, _ := m.findActivityByCaption("NestedReview", 0) + taskPage := dGetDoc(actDoc, "TaskPage") + if taskPage == nil { + t.Fatal("TaskPage should be set on nested activity even when key was absent") + } + if got := dGetString(taskPage, "Page"); got != "MyModule.NestedPage" { + t.Errorf("Page = %q, want MyModule.NestedPage", got) + } + + // Verify parent decision still has its Outcomes intact. + parentDoc, _ := m.findActivityByCaption("Check", 0) + if parentDoc == nil { + t.Fatal("parent decision activity should still exist") + } + if outcomes := dGet(parentDoc, "Outcomes"); outcomes == nil { + t.Fatal("parent decision Outcomes should still be present") } } diff --git a/mdl/executor/widget_registry.go b/mdl/executor/widget_registry.go index defe7b71..701c0bd3 100644 --- a/mdl/executor/widget_registry.go +++ b/mdl/executor/widget_registry.go @@ -16,8 +16,8 @@ import ( // WidgetRegistry holds loaded widget definitions keyed by uppercase MDL name. type WidgetRegistry struct { - byMDLName map[string]*WidgetDefinition // keyed by uppercase MDLName - byWidgetID map[string]*WidgetDefinition // keyed by widgetId + byMDLName map[string]*WidgetDefinition // keyed by uppercase MDLName + byWidgetID map[string]*WidgetDefinition // keyed by widgetId knownOperations map[string]bool // operations accepted during validation } diff --git a/mdl/types/edmx_test.go b/mdl/types/edmx_test.go index a4159e93..01d2b374 100644 --- a/mdl/types/edmx_test.go +++ b/mdl/types/edmx_test.go @@ -217,9 +217,9 @@ func TestFindEntityType(t *testing.T) { func TestResolveNavType(t *testing.T) { tests := []struct { - input string - typeName string - isMany bool + input string + typeName string + isMany bool }{ {"Collection(NS.Order)", "Order", true}, {"NS.Customer", "Customer", false}, diff --git a/mdl/types/id_test.go b/mdl/types/id_test.go index d30a5e39..aa138154 100644 --- a/mdl/types/id_test.go +++ b/mdl/types/id_test.go @@ -156,7 +156,7 @@ func TestValidateID(t *testing.T) { {"AABBCCDD-EEFF-1122-3344-556677889900", true}, {"", false}, {"too-short", false}, - {"a1b2c3d4-e5f6-7890-abcd-ef123456789", false}, // 35 chars + {"a1b2c3d4-e5f6-7890-abcd-ef123456789", false}, // 35 chars {"a1b2c3d4-e5f6-7890-abcd-ef12345678901", false}, // 37 chars {"a1b2c3d4xe5f6-7890-abcd-ef1234567890", false}, // wrong separator {"g1b2c3d4-e5f6-7890-abcd-ef1234567890", false}, // invalid hex