fix(odata-import): skip existing nav associations on re-import (no duplicates)

Re-importing external entities (or running an import script twice) created a
numerically-suffixed duplicate of every OData navigation association on each
run — People's Friends/BestFriend became Friends/Friends2/Friends3, etc. A full
TripPin re-import inflated the module from 8 nav associations to 25.

createNavigationAssociations only ensured association-name *uniqueness*: when a
nav property's association already existed, uniqueAssocName appended a suffix
and created a new one, rather than recognizing it as already-imported. The
modelsdk read doesn't preserve RemoteParentNavigationProperty, so the dedup
can't rely on it alone.

Skip a nav property when the parent entity already has an association for it —
matched by RemoteParentNavigationProperty when available (legacy read) and by
the natural association name (== nav-property name) as the engine-agnostic
fallback. Verified end-to-end: a triple TripPin import now yields 8 nav
associations (was 25), and re-imports create 0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: ako

mdl/executor/cmd_contract.go

@@ -954,13 +954,23 @@ func createNavigationAssociations(
 	}
 
 	// Track associations we've already created to avoid duplicates from
-	// inherited nav properties.
+	// inherited nav properties. existingAssocs is keyed by association name (for
+	// uniqueness/renaming); existingNav is keyed by the OData nav-property name
+	// so a re-import skips a nav property that already has an association instead
+	// of creating a numerically-suffixed duplicate (Friends, Friends2, …). The
+	// nav-property key relies on RemoteParentNavigationProperty, which the
+	// legacy read preserves; the modelsdk read does not, so the natural
+	// association name (== nav-property name) is the fallback skip signal.
 	existingAssocs := make(map[assocKey]bool)
+	existingNav := make(map[assocKey]bool)
 	for _, a := range dm.Associations {
 		// Find parent entity name for this association
 		for _, ent := range dm.Entities {
 			if ent.ID == a.ParentID {
 				existingAssocs[assocKey{ent.Name, a.Name}] = true
+				if a.RemoteParentNavigationProperty != "" {
+					existingNav[assocKey{ent.Name, a.RemoteParentNavigationProperty}] = true
+				}
 				break
 			}
 		}
@@ -1004,6 +1014,17 @@ func createNavigationAssociations(
 					continue
 				}
 
+				// Re-import dedup: if this nav property already has an
+				// association on the parent, skip it — otherwise a second import
+				// creates a numerically-suffixed duplicate (Friends, Friends2,
+				// Friends3 …). Match by the OData nav-property name when the read
+				// preserves it (legacy), else by the natural association name
+				// (== nav-property name; the modelsdk read drops the nav prop).
+				if existingNav[assocKey{parentEnt.Name, np.Name}] ||
+					existingAssocs[assocKey{parentEnt.Name, np.Name}] {
+					continue
+				}
+
 				// An association name must be unique within a module and may
 				// not collide with any entity name (CE0065). When the OData
 				// nav property name collides with an existing entity, append
@@ -1059,6 +1080,7 @@ func createNavigationAssociations(
 					continue
 				}
 				existingAssocs[assocKey{parentEnt.Name, assocName}] = true
+				existingNav[assocKey{parentEnt.Name, np.Name}] = true
 				count++
 			}
 		}

mdl/executor/cmd_contract_test.go

@@ -5,7 +5,9 @@ package executor
 import (
 	"testing"
 
+	"github.com/mendixlabs/mxcli/mdl/backend/mock"
 	"github.com/mendixlabs/mxcli/mdl/types"
+	"github.com/mendixlabs/mxcli/model"
 	"github.com/mendixlabs/mxcli/sdk/domainmodel"
 )
 
@@ -80,3 +82,53 @@ func TestApplyExternalEntityFields_ConservativeCapabilityDefault(t *testing.T) {
 		t.Error("Creatable = false, want true (explicit InsertRestrictions=true)")
 	}
 }
+
+// TestCreateNavigationAssociations_NoDuplicateOnReimport guards against the
+// re-import duplication where each OData nav property spawned a numerically-
+// suffixed copy (Friends, Friends2, Friends3 …) on every import. A nav property
+// that already has an association must be skipped.
+func TestCreateNavigationAssociations_NoDuplicateOnReimport(t *testing.T) {
+	person := &types.EdmEntityType{
+		Name: "Person",
+		NavigationProperties: []*types.EdmNavigationProperty{
+			{Name: "Friends", Type: "Collection(NS.Person)"},
+			{Name: "BestFriend", Type: "NS.Person"},
+		},
+	}
+	doc := &types.EdmxDocument{
+		Schemas:    []*types.EdmSchema{{Namespace: "NS", EntityTypes: []*types.EdmEntityType{person}}},
+		EntitySets: []*types.EdmEntitySet{{Name: "People", EntityType: "NS.Person"}},
+	}
+	people := &domainmodel.Entity{Name: "People", Persistable: true}
+	people.ID = model.ID("ent-people")
+	dm := &domainmodel.DomainModel{Entities: []*domainmodel.Entity{people}}
+	dm.ID = model.ID("dm-1")
+
+	typeByQualified := map[string]*types.EdmEntityType{"NS.Person": person}
+	esMap := map[string]string{"NS.Person": "People"}
+
+	var created []*domainmodel.Association
+	mb := &mock.MockBackend{
+		CreateAssociationFunc: func(_ model.ID, a *domainmodel.Association) error {
+			created = append(created, a)
+			return nil
+		},
+	}
+	ctx, _ := newMockCtx(t, withBackend(mb))
+
+	// First import creates both nav associations.
+	if n := createNavigationAssociations(ctx, dm, doc, typeByQualified, esMap, "TripPin.Client"); n != 2 {
+		t.Fatalf("first import created %d associations, want 2", n)
+	}
+	// Persist them into the domain model, as a re-read between imports would.
+	dm.Associations = append(dm.Associations, created...)
+	countAfterFirst := len(created)
+
+	// Second import must create nothing — no Friends2 / BestFriend2 duplicates.
+	if n := createNavigationAssociations(ctx, dm, doc, typeByQualified, esMap, "TripPin.Client"); n != 0 {
+		t.Errorf("re-import created %d associations, want 0 (duplicates)", n)
+	}
+	if extra := len(created) - countAfterFirst; extra != 0 {
+		t.Errorf("re-import persisted %d new associations via the backend, want 0", extra)
+	}
+}