fix #594: preserve omitted scalar fields on create or modify external entity

mdl-examples/bug-tests/594-or-modify-external-entity-preserve-omitted.mdl

@@ -0,0 +1,61 @@
+-- Bug test: `create or modify external entity` must preserve fields the
+-- statement omits rather than overwriting them with zero values.
+--
+-- Issue #594: the executor (mdl/executor/cmd_odata.go) unconditionally wrote
+-- `existingEntity.X = s.X` for every property. When the MDL statement omitted
+-- `RemoteName`, `s.RemoteName` was the empty string and the existing value
+-- was wiped. The resulting `Rest$ODataRemoteEntitySource` BSON had
+-- `RemoteName: ""`, which Studio Pro's Integration Pane visualiser then
+-- dereferences in `ODataRemoteEntitySource.get_RemoteId()` and NREs with
+-- ArgumentNullException("EntityTypeName") — Studio Pro aliases the BSON
+-- `RemoteName` field as the C# property `EntityTypeName`.
+--
+-- The Go unit test in mdl/executor/cmd_odata_mock_test.go
+-- (TestCreateOrModifyExternalEntity_PreservesOmittedFields_Issue594) covers
+-- the executor logic with a mock backend. This MDL script is the end-to-end
+-- regression test: applying it to a real project must leave a
+-- bug594.RemoteAccount with RemoteName = "Account" after the modify, and
+-- the project must open in Studio Pro without the Integration Pane NRE.
+--
+-- Run with:
+--   mxcli exec mdl-examples/bug-tests/594-or-modify-external-entity-preserve-omitted.mdl -p <project>.mpr
+--   mxcli describe entity bug594.RemoteAccount -p <project>.mpr   # RemoteName must still be 'Account'
+--   then open <project>.mpr in Studio Pro and confirm the Integration Pane renders.
+create module bug594;
+
+create odata client bug594.SalesforceAPI (
+  ODataVersion: OData4,
+  MetadataUrl: 'https://api.salesforce.com/odata/v4/$metadata',
+  timeout: 300
+);
+
+create external entity bug594.RemoteAccount
+from odata client bug594.SalesforceAPI
+(
+  EntitySet: 'Accounts',
+  RemoteName: 'Account',
+  Countable: Yes,
+  Creatable: Yes,
+  Deletable: No,
+  Updatable: No
+)
+(
+  AccountId: string(200),
+  AccountName: string(255)
+);
+
+-- Modify without restating RemoteName / Countable / Creatable. Before the
+-- fix, every omitted property was overwritten with its zero value and the
+-- entity ended up unusable. After the fix, all of them are preserved.
+create or modify external entity bug594.RemoteAccount
+from odata client bug594.SalesforceAPI
+(
+  EntitySet: 'Accounts',
+  AllowCreateChangeLocally: Yes
+)
+(
+  AccountId: string(200),
+  AccountName: string(255)
+);
+
+describe entity bug594.RemoteAccount;

mdl/ast/ast_odata.go

@@ -121,16 +121,21 @@ type DropODataServiceStmt struct {
 func (s *DropODataServiceStmt) isStatement() {}
 
 // CreateExternalEntityStmt represents: CREATE [OR MODIFY] EXTERNAL ENTITY Module.Name FROM ODATA CLIENT Module.Service (...) (attrs);
+//
+// Scalar property fields are pointers so the executor can distinguish
+// "omitted by user" (nil → preserve existing value on CREATE OR MODIFY)
+// from "explicitly set to zero" (e.g. Countable: false). Treating omitted
+// fields as zero on modify silently corrupted entities — see issue #594.
 type CreateExternalEntityStmt struct {
 	Name                     QualifiedName
 	ServiceRef               QualifiedName // FROM ODATA CLIENT ...
-	EntitySet                string
-	RemoteName               string
-	Countable                bool
-	Creatable                bool
-	Deletable                bool
-	Updatable                bool
-	AllowCreateChangeLocally bool        // "Allow creating and changing locally" flag
+	EntitySet                *string
+	RemoteName               *string
+	Countable                *bool
+	Creatable                *bool
+	Deletable                *bool
+	Updatable                *bool
+	AllowCreateChangeLocally *bool       // "Allow creating and changing locally" flag
 	Attributes               []Attribute // reuse from ast_entity.go
 	Documentation            string
 	CreateOrModify           bool

mdl/executor/cmd_odata.go

@@ -806,16 +806,34 @@ func execCreateExternalEntity(ctx *ExecContext, s *ast.CreateExternalEntityStmt)
 	serviceRef := s.ServiceRef.String()
 
 	if existingEntity != nil {
-		// Update existing entity
+		// Update existing entity. Issue #594: only assign fields that the
+		// MDL statement explicitly set. Omitted (nil) fields preserve the
+		// prior value — previously the executor unconditionally wrote zero
+		// values, wiping fields like RemoteName and triggering Studio Pro
+		// NREs (e.g. ODataRemoteEntitySource.RemoteId on empty RemoteName).
 		existingEntity.Source = "Rest$ODataRemoteEntitySource"
 		existingEntity.RemoteServiceName = serviceRef
-		existingEntity.RemoteEntitySet = s.EntitySet
-		existingEntity.RemoteEntityName = s.RemoteName
-		existingEntity.Countable = s.Countable
-		existingEntity.Creatable = s.Creatable
-		existingEntity.Deletable = s.Deletable
-		existingEntity.Updatable = s.Updatable
-		existingEntity.CreateChangeLocally = s.AllowCreateChangeLocally
+		if s.EntitySet != nil {
+			existingEntity.RemoteEntitySet = *s.EntitySet
+		}
+		if s.RemoteName != nil {
+			existingEntity.RemoteEntityName = *s.RemoteName
+		}
+		if s.Countable != nil {
+			existingEntity.Countable = *s.Countable
+		}
+		if s.Creatable != nil {
+			existingEntity.Creatable = *s.Creatable
+		}
+		if s.Deletable != nil {
+			existingEntity.Deletable = *s.Deletable
+		}
+		if s.Updatable != nil {
+			existingEntity.Updatable = *s.Updatable
+		}
+		if s.AllowCreateChangeLocally != nil {
+			existingEntity.CreateChangeLocally = *s.AllowCreateChangeLocally
+		}
 		if len(attrs) > 0 {
 			existingEntity.Attributes = attrs
 		}
@@ -829,6 +847,11 @@ func execCreateExternalEntity(ctx *ExecContext, s *ast.CreateExternalEntityStmt)
 		return nil
 	}
 
+	// Initial create: EntitySet is required (no prior value to preserve).
+	if s.EntitySet == nil || *s.EntitySet == "" {
+		return mdlerrors.NewValidation("EntitySet property is required when creating an external entity")
+	}
+
 	// Auto-position based on existing entities
 	location := model.Point{X: 100 + len(dm.Entities)*150, Y: 100}
 
@@ -840,13 +863,13 @@ func execCreateExternalEntity(ctx *ExecContext, s *ast.CreateExternalEntityStmt)
 		Attributes:          attrs,
 		Source:              "Rest$ODataRemoteEntitySource",
 		RemoteServiceName:   serviceRef,
-		RemoteEntitySet:     s.EntitySet,
-		RemoteEntityName:    s.RemoteName,
-		Countable:           s.Countable,
-		Creatable:           s.Creatable,
-		Deletable:           s.Deletable,
-		Updatable:           s.Updatable,
-		CreateChangeLocally: s.AllowCreateChangeLocally,
+		RemoteEntitySet:     *s.EntitySet,
+		RemoteEntityName:    derefString(s.RemoteName),
+		Countable:           derefBool(s.Countable),
+		Creatable:           derefBool(s.Creatable),
+		Deletable:           derefBool(s.Deletable),
+		Updatable:           derefBool(s.Updatable),
+		CreateChangeLocally: derefBool(s.AllowCreateChangeLocally),
 	}
 	newEntity.ID = model.ID(types.GenerateID())
 
@@ -857,6 +880,20 @@ func execCreateExternalEntity(ctx *ExecContext, s *ast.CreateExternalEntityStmt)
 	return nil
 }
 
+func derefString(p *string) string {
+	if p == nil {
+		return ""
+	}
+	return *p
+}
+
+func derefBool(p *bool) bool {
+	if p == nil {
+		return false
+	}
+	return *p
+}
+
 // ============================================================================
 // OData Write Handlers (CREATE / ALTER / DROP)
 // ============================================================================

mdl/executor/cmd_odata_mock_test.go

@@ -12,6 +12,9 @@ import (
 	"github.com/mendixlabs/mxcli/sdk/domainmodel"
 )
 
+func strPtr(s string) *string { return &s }
+func boolPtr(b bool) *bool    { return &b }
+
 func TestShowODataClients_Mock(t *testing.T) {
 	mod := mkModule("MyModule")
 	svc := &model.ConsumedODataService{
@@ -227,7 +230,7 @@ func TestCreateExternalEntity_RejectsNonExistentClient(t *testing.T) {
 	stmt := &ast.CreateExternalEntityStmt{
 		Name:       ast.QualifiedName{Module: "MyModule", Name: "FakeEntity"},
 		ServiceRef: ast.QualifiedName{Module: "MyModule", Name: "NonExistentClient"},
-		EntitySet:  "Products",
+		EntitySet:  strPtr("Products"),
 	}
 	err := execCreateExternalEntity(ctx, stmt)
 	assertError(t, err)
@@ -267,7 +270,7 @@ func TestCreateExternalEntity_AcceptsExistingClient(t *testing.T) {
 	stmt := &ast.CreateExternalEntityStmt{
 		Name:       ast.QualifiedName{Module: "MyModule", Name: "Product"},
 		ServiceRef: ast.QualifiedName{Module: "MyModule", Name: "ProductsClient"},
-		EntitySet:  "Products",
+		EntitySet:  strPtr("Products"),
 	}
 	assertNoError(t, execCreateExternalEntity(ctx, stmt))
 	if created == nil {
@@ -310,8 +313,8 @@ func TestCreateExternalEntity_AllowCreateChangeLocally_Issue534(t *testing.T) {
 	stmt := &ast.CreateExternalEntityStmt{
 		Name:                     ast.QualifiedName{Module: "TripPin", Name: "People"},
 		ServiceRef:               ast.QualifiedName{Module: "TripPin", Name: "TripPinClient"},
-		EntitySet:                "People",
-		AllowCreateChangeLocally: true,
+		EntitySet:                strPtr("People"),
+		AllowCreateChangeLocally: boolPtr(true),
 	}
 	assertNoError(t, execCreateExternalEntity(ctx, stmt))
 	if created == nil {
@@ -322,6 +325,98 @@ func TestCreateExternalEntity_AllowCreateChangeLocally_Issue534(t *testing.T) {
 	}
 }
 
+// TestCreateOrModifyExternalEntity_PreservesOmittedFields_Issue594 verifies that
+// CREATE OR MODIFY EXTERNAL ENTITY preserves the existing value of any field
+// that is omitted from the MDL statement, rather than overwriting it with the
+// zero value. Previously the executor unconditionally wrote `existingEntity.X = s.X`
+// for every field, so omitting `RemoteName` on `or modify` wiped the BSON
+// `RemoteName` to "" and triggered Studio Pro's
+// ODataRemoteEntitySource.get_RemoteId() NRE in the Integration pane.
+func TestCreateOrModifyExternalEntity_PreservesOmittedFields_Issue594(t *testing.T) {
+	mod := mkModule("OdTest")
+	h := mkHierarchy(mod)
+	svc := &model.ConsumedODataService{
+		BaseElement: model.BaseElement{ID: nextID("cos")},
+		ContainerID: mod.ID,
+		Name:        "SalesforceAPI",
+	}
+	withContainer(h, svc.ContainerID, mod.ID)
+
+	// Pre-existing external entity with non-default values for every field
+	// that the bug could clobber.
+	existing := &domainmodel.Entity{
+		BaseElement:         model.BaseElement{ID: nextID("ent")},
+		Name:                "RemoteAccount",
+		Source:              "Rest$ODataRemoteEntitySource",
+		RemoteServiceName:   "OdTest.SalesforceAPI",
+		RemoteEntitySet:     "Accounts",
+		RemoteEntityName:    "Account", // <-- the field that #594 wiped
+		Countable:           true,
+		Creatable:           true,
+		Deletable:           true,
+		Updatable:           true,
+		CreateChangeLocally: false,
+	}
+	dm := &domainmodel.DomainModel{
+		BaseElement: model.BaseElement{ID: nextID("dm")},
+		ContainerID: mod.ID,
+		Entities:    []*domainmodel.Entity{existing},
+	}
+
+	var updated *domainmodel.Entity
+	mb := &mock.MockBackend{
+		IsConnectedFunc: func() bool { return true },
+		ListModulesFunc: func() ([]*model.Module, error) {
+			return []*model.Module{mod}, nil
+		},
+		GetDomainModelFunc: func(id model.ID) (*domainmodel.DomainModel, error) {
+			return dm, nil
+		},
+		ListConsumedODataServicesFunc: func() ([]*model.ConsumedODataService, error) {
+			return []*model.ConsumedODataService{svc}, nil
+		},
+		UpdateEntityFunc: func(dmID model.ID, entity *domainmodel.Entity) error {
+			updated = entity
+			return nil
+		},
+	}
+
+	ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h))
+
+	// Only mention EntitySet + AllowCreateChangeLocally — every other
+	// scalar should be preserved from the existing entity, not zeroed.
+	stmt := &ast.CreateExternalEntityStmt{
+		Name:                     ast.QualifiedName{Module: "OdTest", Name: "RemoteAccount"},
+		ServiceRef:               ast.QualifiedName{Module: "OdTest", Name: "SalesforceAPI"},
+		EntitySet:                strPtr("Accounts"),
+		AllowCreateChangeLocally: boolPtr(true),
+		CreateOrModify:           true,
+	}
+	assertNoError(t, execCreateExternalEntity(ctx, stmt))
+
+	if updated == nil {
+		t.Fatal("expected UpdateEntity to be called")
+	}
+	if updated.RemoteEntityName != "Account" {
+		t.Errorf("RemoteEntityName = %q, want \"Account\" (omitted RemoteName must preserve existing value)", updated.RemoteEntityName)
+	}
+	if !updated.Countable {
+		t.Errorf("Countable = false, want true (omitted Countable must preserve existing value)")
+	}
+	if !updated.Creatable {
+		t.Errorf("Creatable = false, want true (omitted Creatable must preserve existing value)")
+	}
+	if !updated.Deletable {
+		t.Errorf("Deletable = false, want true (omitted Deletable must preserve existing value)")
+	}
+	if !updated.Updatable {
+		t.Errorf("Updatable = false, want true (omitted Updatable must preserve existing value)")
+	}
+	if !updated.CreateChangeLocally {
+		t.Errorf("CreateChangeLocally = false, want true (explicitly set value)")
+	}
+}
+
 // TestDescribeODataService_ExposeRoundtrip verifies that DESCRIBE ODATA SERVICE
 // output for entities with key/filterable/sortable members is valid MDL that
 // the parser can re-parse (issue #400).

mdl/visitor/visitor_odata.go

@@ -165,21 +165,22 @@ func (b *Builder) ExitCreateExternalEntityStatement(ctx *parser.CreateExternalEn
 		value := odataAssignmentValueText(prop)
 
 		boolVal := strings.EqualFold(value, "true") || strings.EqualFold(value, "yes")
+		strVal := value
 		switch strings.ToLower(name) {
 		case "entityset":
-			stmt.EntitySet = value
+			stmt.EntitySet = &strVal
 		case "remotename":
-			stmt.RemoteName = value
+			stmt.RemoteName = &strVal
 		case "countable":
-			stmt.Countable = boolVal
+			stmt.Countable = &boolVal
 		case "creatable":
-			stmt.Creatable = boolVal
+			stmt.Creatable = &boolVal
 		case "deletable":
-			stmt.Deletable = boolVal
+			stmt.Deletable = &boolVal
 		case "updatable":
-			stmt.Updatable = boolVal
+			stmt.Updatable = &boolVal
 		case "allowcreatechangelocally", "allowcreatingandchanginglocally", "createchangelocally":
-			stmt.AllowCreateChangeLocally = boolVal
+			stmt.AllowCreateChangeLocally = &boolVal
 		}
 	}
 

mdl/visitor/visitor_odata_test.go

@@ -96,11 +96,14 @@ func TestCreateExternalEntity(t *testing.T) {
 	if stmt.ServiceRef.Name != "PetStore" {
 		t.Errorf("Got ServiceRef %s", stmt.ServiceRef.Name)
 	}
-	if stmt.EntitySet != "Products" {
-		t.Errorf("Got EntitySet %q", stmt.EntitySet)
+	if stmt.EntitySet == nil || *stmt.EntitySet != "Products" {
+		t.Errorf("Got EntitySet %v", stmt.EntitySet)
 	}
-	if !stmt.Countable {
-		t.Error("Expected Countable true")
+	if stmt.RemoteName == nil || *stmt.RemoteName != "Product" {
+		t.Errorf("Got RemoteName %v", stmt.RemoteName)
+	}
+	if stmt.Countable == nil || !*stmt.Countable {
+		t.Errorf("Expected Countable true, got %v", stmt.Countable)
 	}
 	if len(stmt.Attributes) != 2 {
 		t.Errorf("Expected 2 attributes, got %d", len(stmt.Attributes))