Skip to content

Commit 3c22598

Browse files
akoclaude
andcommitted
Fix DATAGRID placeholder ID leak during template augmentation (issue #6)
regenerateNestedIDs(vt) was called after capturing newVTID = vt["$ID"], overwriting the VT's $ID with a different placeholder. The Object's Value.TypePointer still referenced the original newVTID, which was never collected by collectIDs and leaked into the final BSON. Fix: call regenerateNestedIDs before setting vt["$ID"] so the authoritative ID isn't overwritten. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent ccd8f17 commit 3c22598

2 files changed

Lines changed: 87 additions & 3 deletions

File tree

sdk/widgets/augment.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,13 @@ func clonePropertyPair(propTypes []any, objProps []any, exemplarIdx int, p mpk.P
190190
// Update the ValueType ID and set defaults
191191
var newVTID string
192192
if vt, ok := getMapField(newPT, "ValueType"); ok {
193+
// Regenerate nested $ID fields FIRST (EnumerationValues, ObjectType, etc.)
194+
// so they get unique placeholders without overwriting the IDs we set below.
195+
regenerateNestedIDs(vt)
196+
197+
// Now set the top-level VT $ID — this must happen AFTER regenerateNestedIDs
198+
// because regenerateNestedIDs replaces ALL $ID fields including this one.
199+
// The Property's Value.TypePointer will reference this ID, so it must match.
193200
newVTID = placeholderID()
194201
vt["$ID"] = newVTID
195202

@@ -226,9 +233,6 @@ func clonePropertyPair(propTypes []any, objProps []any, exemplarIdx int, p mpk.P
226233
if vtType != "Expression" {
227234
vt["ReturnType"] = nil
228235
}
229-
230-
// Regenerate any nested $ID fields
231-
regenerateNestedIDs(vt)
232236
}
233237

234238
// Find the corresponding Property in objProps that uses the exemplar's TypePointer

sdk/widgets/augment_test.go

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
package widgets
44

55
import (
6+
"fmt"
67
"testing"
78

89
"github.com/mendixlabs/mxcli/sdk/widgets/mpk"
@@ -573,6 +574,85 @@ func TestAugmentTemplate_WithRealTemplate(t *testing.T) {
573574
}
574575
}
575576

577+
// TestAugmentTemplate_NoPlaceholderLeakAfterBSONConversion verifies that after
578+
// augmentation and BSON conversion, no placeholder IDs remain. This is the bug
579+
// from issue #6: regenerateNestedIDs overwrote ValueType.$ID after newVTID was
580+
// captured, causing Value.TypePointer to reference an unmapped placeholder.
581+
func TestAugmentTemplate_NoPlaceholderLeakAfterBSONConversion(t *testing.T) {
582+
ResetPlaceholderCounter()
583+
584+
tmpl, err := GetTemplate("com.mendix.widget.web.combobox.Combobox")
585+
if err != nil {
586+
t.Fatalf("GetTemplate failed: %v", err)
587+
}
588+
if tmpl == nil {
589+
t.Skip("ComboBox template not available")
590+
}
591+
592+
clone := deepCloneTemplate(tmpl)
593+
594+
// Build a definition with existing properties + one new one
595+
objType := clone.Type["ObjectType"].(map[string]any)
596+
propTypes := objType["PropertyTypes"].([]any)
597+
def := &mpk.WidgetDefinition{
598+
ID: "com.mendix.widget.web.combobox.Combobox",
599+
Version: "3.0.0",
600+
}
601+
for _, pt := range propTypes {
602+
ptMap, ok := pt.(map[string]any)
603+
if !ok {
604+
continue
605+
}
606+
key, _ := ptMap["PropertyKey"].(string)
607+
vt, _ := ptMap["ValueType"].(map[string]any)
608+
vtType := ""
609+
if vt != nil {
610+
vtType, _ = vt["Type"].(string)
611+
}
612+
if vtType == "System" {
613+
def.SystemProps = append(def.SystemProps, mpk.PropertyDef{Key: key, IsSystem: true})
614+
} else {
615+
xmlType := bsonTypeToXmlType(vtType)
616+
def.Properties = append(def.Properties, mpk.PropertyDef{Key: key, Type: xmlType})
617+
}
618+
}
619+
def.Properties = append(def.Properties, mpk.PropertyDef{
620+
Key: "extraBoolProp",
621+
Type: "boolean",
622+
Caption: "Extra Bool",
623+
DefaultValue: "false",
624+
})
625+
626+
if err := AugmentTemplate(clone, def); err != nil {
627+
t.Fatalf("AugmentTemplate failed: %v", err)
628+
}
629+
630+
// Now run the full BSON conversion pipeline (same as GetTemplateFullBSON)
631+
counter := 0
632+
idGen := func() string {
633+
counter++
634+
return fmt.Sprintf("bbbbbbbb0000000000000000%08x", counter)
635+
}
636+
637+
idMapping := make(map[string]string)
638+
collectIDs(clone.Type, idGen, idMapping)
639+
if clone.Object != nil {
640+
collectIDs(clone.Object, idGen, idMapping)
641+
}
642+
643+
var objectTypeID string
644+
propertyTypeIDs := make(map[string]PropertyTypeIDEntry)
645+
bsonType := jsonToBSONWithMappingAndObjectType(clone.Type, idMapping, propertyTypeIDs, &objectTypeID)
646+
bsonObject := jsonToBSONObjectWithMapping(clone.Object, idMapping)
647+
648+
if containsPlaceholderID(bsonType) {
649+
t.Error("placeholder ID leak in Type BSON after augmentation")
650+
}
651+
if containsPlaceholderID(bsonObject) {
652+
t.Error("placeholder ID leak in Object BSON after augmentation — issue #6 regression")
653+
}
654+
}
655+
576656
// bsonTypeToXmlType is a test helper to reverse the mapping.
577657
func bsonTypeToXmlType(bsonType string) string {
578658
switch bsonType {

0 commit comments

Comments
 (0)