OCPCRT-641: Fix two more hierarchy name-collision defects

go/hierarchy_test.go

@@ -1,6 +1,8 @@
 package orgdatacore
 
 import (
+	"context"
+	"encoding/json"
 	"testing"
 )
 
@@ -165,6 +167,22 @@ func TestGetHierarchyPathNameCollision(t *testing.T) {
 	if !service.IsEmployeeInOrg("euser", "acme") {
 		t.Error("Expected euser to be in org 'acme' via the name-colliding team hierarchy")
 	}
+
+	// Reciprocal: requesting the same name with the team_group type must
+	// resolve the team_group, not be rejected because a team shares the name.
+	tgPath := service.GetHierarchyPath("shared", "team_group")
+	tgExpected := []HierarchyPathEntry{
+		{Name: "shared", Type: "team_group"},
+		{Name: "acme", Type: "org"},
+	}
+	if len(tgPath) != len(tgExpected) {
+		t.Fatalf("team_group path: expected %d entries, got %d: %+v", len(tgExpected), len(tgPath), tgPath)
+	}
+	for i, exp := range tgExpected {
+		if tgPath[i] != exp {
+			t.Errorf("team_group path entry %d: expected %+v, got %+v", i, exp, tgPath[i])
+		}
+	}
 }
 
 func TestGetHierarchyPathNoData(t *testing.T) {
@@ -303,3 +321,107 @@ func TestGetDescendantsTreeNoData(t *testing.T) {
 		t.Error("Expected nil tree when no data loaded")
 	}
 }
+
+// TestGetDescendantsTreeNameCollision covers a team_group and a team that share
+// a name. The tree must key both the children map and the visited set by
+// (name, type): keying by name alone merges the two nodes' children and stops
+// the recursion early, producing a wrong tree.
+func TestGetDescendantsTreeNameCollision(t *testing.T) {
+	service := NewService()
+	service.data = &Data{
+		Lookups: Lookups{
+			Orgs: map[string]Org{"acme": {Name: "acme", Type: "org"}},
+			TeamGroups: map[string]TeamGroup{
+				"shared": {Name: "shared", Type: "team_group", Parent: &ParentInfo{Name: "acme", Type: "org"}},
+			},
+			Teams: map[string]Team{
+				"shared": {Name: "shared", Type: "team", Parent: &ParentInfo{Name: "shared", Type: "team_group"}},
+				"leaf":   {Name: "leaf", Type: "team", Parent: &ParentInfo{Name: "shared", Type: "team"}},
+			},
+		},
+	}
+
+	// acme(org) -> shared(team_group) -> shared(team) -> leaf(team)
+	tree := service.GetDescendantsTree("acme")
+	if tree == nil {
+		t.Fatal("Expected non-nil tree")
+	}
+	if len(tree.Children) != 1 || tree.Children[0].Name != "shared" || tree.Children[0].Type != "team_group" {
+		t.Fatalf("Expected acme's only child to be shared/team_group, got %+v", tree.Children)
+	}
+	tg := tree.Children[0]
+	if len(tg.Children) != 1 || tg.Children[0].Type != "team" {
+		t.Fatalf("Expected team_group shared's only child to be shared/team, got %+v", tg.Children)
+	}
+	team := tg.Children[0]
+	if len(team.Children) != 1 || team.Children[0].Name != "leaf" {
+		t.Fatalf("Expected team shared's only child to be leaf, got %+v", team.Children)
+	}
+}
+
+// TestMixedCaseTypesNormalizedAtLoad verifies hierarchy types are canonicalized
+// to lowercase at the load boundary, so traversal resolves regardless of the
+// casing the data source used for entity and parent types.
+func TestMixedCaseTypesNormalizedAtLoad(t *testing.T) {
+	data := &Data{
+		Lookups: Lookups{
+			Employees: map[string]Employee{"u1": {UID: "u1", Email: "u1@example.com"}},
+			Orgs:      map[string]Org{"acme": {Name: "acme", Type: "ORG"}},
+			TeamGroups: map[string]TeamGroup{
+				"grp": {Name: "grp", Type: "Team_Group", Parent: &ParentInfo{Name: "acme", Type: "ORG"}},
+			},
+			Teams: map[string]Team{
+				"t": {Name: "t", Type: "TEAM", Parent: &ParentInfo{Name: "grp", Type: "TEAM_GROUP"}},
+			},
+		},
+		Indexes: Indexes{
+			Membership: MembershipIndex{MembershipIndex: map[string][]MembershipInfo{
+				"u1": {{Name: "t", Type: "TEAM"}},
+			}},
+		},
+	}
+	jsonData, err := json.Marshal(data)
+	if err != nil {
+		t.Fatalf("marshal: %v", err)
+	}
+	service := NewService()
+	if err := service.LoadFromDataSource(context.Background(), NewFakeDataSource(string(jsonData))); err != nil {
+		t.Fatalf("load: %v", err)
+	}
+
+	// Descendants resolve: the children map keys on parent type "ORG"/"TEAM_GROUP"
+	// which, once normalized, match the canonical lowercase child types.
+	tree := service.GetDescendantsTree("acme")
+	if tree == nil {
+		t.Fatal("Expected non-nil tree")
+	}
+	if len(tree.Children) != 1 || tree.Children[0].Name != "grp" {
+		t.Fatalf("Expected acme's only child to be grp, got %+v", tree.Children)
+	}
+	grp := tree.Children[0]
+	if len(grp.Children) != 1 || grp.Children[0].Name != "t" {
+		t.Fatalf("Expected grp's only child to be t, got %+v", grp.Children)
+	}
+
+	// Hierarchy path entries carry canonical lowercase types (from normalized
+	// parent refs), so consumers need not defensively lowercase them. A
+	// caller-supplied type in any casing normalizes to the same canonical path,
+	// including the first entry (which is derived from the caller's argument).
+	wantTypes := []string{"team", "team_group", "org"}
+	for _, callerType := range []string{"team", "TEAM", "Team"} {
+		path := service.GetHierarchyPath("t", callerType)
+		if len(path) != len(wantTypes) {
+			t.Fatalf("GetHierarchyPath(t, %q): expected %d path entries, got %+v", callerType, len(wantTypes), path)
+		}
+		for i, wt := range wantTypes {
+			if path[i].Type != wt {
+				t.Errorf("GetHierarchyPath(t, %q): path[%d].Type = %q, want %q", callerType, i, path[i].Type, wt)
+			}
+		}
+	}
+
+	// Entity own types are normalized in the lookups too.
+	if tm := service.GetTeamByName("t"); tm == nil || tm.Type != "team" {
+		t.Errorf("expected team 't' type normalized to 'team', got %+v", tm)
+	}
+}

go/organization_test.go

@@ -311,3 +311,49 @@ func TestOrgInfoTypes(t *testing.T) {
 		}
 	}
 }
+
+// TestGetUserOrganizationsNameCollision covers a hierarchy where a team shares
+// its parent team_group's name. Deduping the result by (name, type) — not name
+// alone — is required, otherwise the team_group is wrongly dropped because a
+// team with the same name was already recorded.
+func TestGetUserOrganizationsNameCollision(t *testing.T) {
+	service := NewService()
+	service.data = &Data{
+		Lookups: Lookups{
+			Teams: map[string]Team{
+				"shared": {Name: "shared", Type: "team", Parent: &ParentInfo{Name: "shared", Type: "team_group"}},
+			},
+			TeamGroups: map[string]TeamGroup{
+				"shared": {Name: "shared", Type: "team_group", Parent: &ParentInfo{Name: "acme", Type: "org"}},
+			},
+			Orgs: map[string]Org{"acme": {Name: "acme", Type: "org"}},
+		},
+		Indexes: Indexes{
+			Membership: MembershipIndex{MembershipIndex: map[string][]MembershipInfo{
+				"euser": {{Name: "shared", Type: "team"}},
+			}},
+			SlackIDMappings: SlackIDMappings{SlackUIDToUID: map[string]string{"Suser": "euser"}},
+		},
+	}
+
+	// euser is a member of team "shared" -> team_group "shared" -> org "acme".
+	// All three must appear, each with its own type.
+	expected := []OrgInfo{
+		{Name: "shared", Type: OrgTypeTeam},
+		{Name: "shared", Type: OrgTypeTeamGroup},
+		{Name: "acme", Type: OrgTypeOrganization},
+	}
+	result := service.GetUserOrganizations("Suser")
+	for _, exp := range expected {
+		found := false
+		for _, actual := range result {
+			if actual.Name == exp.Name && actual.Type == exp.Type {
+				found = true
+				break
+			}
+		}
+		if !found {
+			t.Errorf("GetUserOrganizations missing %+v; got %+v", exp, result)
+		}
+	}
+}