Skip to content

Commit 00b2616

Browse files
committed
optimize collaborators feature implementation
- Combine duplicate API calls: merge repoInvitationsWithPermissions and repoInvitationIDs into single repoInvitationsData function - Remove unnecessary helper methods: replace has() and get() with direct map access - Improve data structure: use single map with struct instead of two separate maps for better memory locality - Tighten variable scoping: use conditional assignment pattern for cleaner code - Rename for clarity: RepoInvitation -> CollaboratorRepoInvitation (same for update, delete methods), ID -> InvitationID - Add GitHub API documentation links
1 parent 5a35864 commit 00b2616

File tree

6 files changed

+129
-150
lines changed

6 files changed

+129
-150
lines changed

cmd/peribolos/main.go

Lines changed: 46 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -361,15 +361,6 @@ func dumpOrgConfig(client dumpClient, orgName string, ignoreSecretTeams bool, ap
361361
}
362362
logrus.WithField("repo", full.FullName).Debug("Recording repo.")
363363

364-
// Get direct collaborators (explicitly added) via GraphQL
365-
var collaborators map[string]github.RepoPermissionLevel
366-
directCollabs, err := client.ListDirectCollaboratorsWithPermissions(orgName, repo.Name)
367-
if err != nil {
368-
logrus.WithError(err).Warnf("Failed to list direct collaborators for %s/%s", orgName, repo.Name)
369-
} else if len(directCollabs) > 0 {
370-
collaborators = directCollabs
371-
}
372-
373364
repoConfig := org.PruneRepoDefaults(org.Repo{
374365
Description: &full.Description,
375366
HomePage: &full.Homepage,
@@ -382,8 +373,15 @@ func dumpOrgConfig(client dumpClient, orgName string, ignoreSecretTeams bool, ap
382373
AllowRebaseMerge: &full.AllowRebaseMerge,
383374
Archived: &full.Archived,
384375
DefaultBranch: &full.DefaultBranch,
385-
Collaborators: collaborators,
376+
// Collaborators will be set conditionally below
386377
})
378+
379+
// Get direct collaborators (explicitly added) via GraphQL
380+
if directCollabs, err := client.ListDirectCollaboratorsWithPermissions(orgName, repo.Name); err != nil {
381+
logrus.WithError(err).Warnf("Failed to list direct collaborators for %s/%s", orgName, repo.Name)
382+
} else if len(directCollabs) > 0 {
383+
repoConfig.Collaborators = directCollabs
384+
}
387385
out.Repos[full.Name] = repoConfig
388386
}
389387

@@ -532,78 +530,62 @@ func normalize(s sets.Set[string]) sets.Set[string] {
532530
return out
533531
}
534532

533+
// collaboratorInfo holds permission and original username for a normalized user
534+
type collaboratorInfo struct {
535+
permission github.RepoPermissionLevel
536+
originalName string
537+
}
538+
535539
// collaboratorMap manages collaborator usernames to permissions with normalization support
536540
type collaboratorMap struct {
537-
permissions map[string]github.RepoPermissionLevel // normalized_username -> permission
538-
origNames map[string]string // normalized_username -> original_username
541+
collaborators map[string]collaboratorInfo // normalized_username -> collaborator info
539542
}
540543

541544
// newCollaboratorMap creates a collaborator map from a raw username->permission map
542545
func newCollaboratorMap(raw map[string]github.RepoPermissionLevel) collaboratorMap {
543546
cm := collaboratorMap{
544-
permissions: make(map[string]github.RepoPermissionLevel, len(raw)),
545-
origNames: make(map[string]string, len(raw)),
547+
collaborators: make(map[string]collaboratorInfo, len(raw)),
546548
}
547549
for username, permission := range raw {
548550
normalized := github.NormLogin(username)
549-
cm.permissions[normalized] = permission
550-
cm.origNames[normalized] = username
551+
cm.collaborators[normalized] = collaboratorInfo{
552+
permission: permission,
553+
originalName: username,
554+
}
551555
}
552556
return cm
553557
}
554558

555-
// has checks if a normalized username exists in the map
556-
func (cm collaboratorMap) has(normalizedUser string) bool {
557-
_, exists := cm.permissions[normalizedUser]
558-
return exists
559-
}
560-
561-
// get returns the permission for a normalized username
562-
func (cm collaboratorMap) get(normalizedUser string) (github.RepoPermissionLevel, bool) {
563-
perm, exists := cm.permissions[normalizedUser]
564-
return perm, exists
565-
}
566-
567559
// originalName returns the original casing for a normalized username
568560
func (cm collaboratorMap) originalName(normalizedUser string) string {
569-
return cm.origNames[normalizedUser]
561+
return cm.collaborators[normalizedUser].originalName
570562
}
571563

572564
func (m *memberships) normalize() {
573565
m.members = normalize(m.members)
574566
m.super = normalize(m.super)
575567
}
576568

577-
// repoInvitationsWithPermissions returns pending repository invitations with their permission levels
578-
func repoInvitationsWithPermissions(client collaboratorClient, orgName, repoName string) (map[string]github.RepoPermissionLevel, error) {
579-
invitations := map[string]github.RepoPermissionLevel{}
580-
is, err := client.ListRepoInvitations(orgName, repoName)
581-
if err != nil {
582-
return nil, err
583-
}
584-
for _, i := range is {
585-
if i.Invitee == nil || i.Invitee.Login == "" {
586-
continue
587-
}
588-
invitations[github.NormLogin(i.Invitee.Login)] = i.Permission
589-
}
590-
return invitations, nil
591-
}
592-
593-
// repoInvitationIDs returns a mapping of normalized usernames to invitation IDs
594-
func repoInvitationIDs(client collaboratorClient, orgName, repoName string) (map[string]int, error) {
569+
// repoInvitationsData returns pending repository invitations with both permissions and IDs
570+
func repoInvitationsData(client collaboratorClient, orgName, repoName string) (map[string]github.RepoPermissionLevel, map[string]int, error) {
571+
permissions := map[string]github.RepoPermissionLevel{}
595572
invitationIDs := map[string]int{}
573+
596574
is, err := client.ListRepoInvitations(orgName, repoName)
597575
if err != nil {
598-
return nil, err
576+
return nil, nil, err
599577
}
578+
600579
for _, i := range is {
601580
if i.Invitee == nil || i.Invitee.Login == "" {
602581
continue
603582
}
604-
invitationIDs[github.NormLogin(i.Invitee.Login)] = i.ID
583+
normalizedLogin := github.NormLogin(i.Invitee.Login)
584+
permissions[normalizedLogin] = i.Permission
585+
invitationIDs[normalizedLogin] = i.InvitationID
605586
}
606-
return invitationIDs, nil
587+
588+
return permissions, invitationIDs, nil
607589
}
608590

609591
func configureMembers(have, want memberships, invitees sets.Set[string], adder func(user string, super bool) error, remover func(user string) error) error {
@@ -1160,11 +1142,11 @@ type collaboratorClient interface {
11601142
ListDirectCollaboratorsWithPermissions(org, repo string) (map[string]github.RepoPermissionLevel, error)
11611143
AddCollaborator(org, repo, user string, permission github.RepoPermissionLevel) error
11621144
UpdateCollaborator(org, repo, user string, permission github.RepoPermissionLevel) error
1163-
UpdateRepoInvitation(org, repo string, invitationID int, permission github.RepoPermissionLevel) error
1164-
DeleteRepoInvitation(org, repo string, invitationID int) error
1145+
UpdateCollaboratorRepoInvitation(org, repo string, invitationID int, permission github.RepoPermissionLevel) error
1146+
DeleteCollaboratorRepoInvitation(org, repo string, invitationID int) error
11651147
RemoveCollaborator(org, repo, user string) error
11661148
UpdateCollaboratorPermission(org, repo, user string, permission github.RepoPermissionLevel) error
1167-
ListRepoInvitations(org, repo string) ([]github.RepoInvitation, error)
1149+
ListRepoInvitations(org, repo string) ([]github.CollaboratorRepoInvitation, error)
11681150
}
11691151

11701152
// configureCollaborators updates the list of repository collaborators when necessary
@@ -1183,18 +1165,12 @@ func configureCollaborators(client collaboratorClient, orgName, repoName string,
11831165
}
11841166
logrus.Debugf("Found %d direct collaborators", len(currentCollaboratorsRaw))
11851167

1186-
// Get pending repository invitations with their permission levels
1187-
pendingInvitations, err := repoInvitationsWithPermissions(client, orgName, repoName)
1168+
// Get pending repository invitations with their permission levels and IDs
1169+
pendingInvitations, pendingInvitationIDs, err := repoInvitationsData(client, orgName, repoName)
11881170
if err != nil {
11891171
logrus.WithError(err).Warnf("Failed to list repository invitations for %s/%s, may send duplicate invitations", orgName, repoName)
11901172
pendingInvitations = map[string]github.RepoPermissionLevel{} // Continue with empty map
1191-
}
1192-
1193-
// Get pending repository invitation IDs for updating invitations
1194-
pendingInvitationIDs, err := repoInvitationIDs(client, orgName, repoName)
1195-
if err != nil {
1196-
logrus.WithError(err).Warnf("Failed to get repository invitation IDs for %s/%s, invitation updates may fail", orgName, repoName)
1197-
pendingInvitationIDs = map[string]int{} // Continue with empty map
1173+
pendingInvitationIDs = map[string]int{} // Continue with empty map
11981174
}
11991175

12001176
// Create combined map of current direct collaborators + pending invitations
@@ -1220,10 +1196,11 @@ func configureCollaborators(client collaboratorClient, orgName, repoName string,
12201196
wantedCollaborators := newCollaboratorMap(want)
12211197

12221198
// Add or update permissions for users in our config
1223-
for normalizedUser, wantPermission := range wantedCollaborators.permissions {
1199+
for normalizedUser, collaboratorInfo := range wantedCollaborators.collaborators {
1200+
wantPermission := collaboratorInfo.permission
12241201
wantUser := wantedCollaborators.originalName(normalizedUser)
12251202

1226-
if currentPermission, exists := currentCollaborators.get(normalizedUser); exists && currentPermission == wantPermission {
1203+
if currentInfo, exists := currentCollaborators.collaborators[normalizedUser]; exists && currentInfo.permission == wantPermission {
12271204
// Permission is already correct
12281205
continue
12291206
}
@@ -1238,7 +1215,7 @@ func configureCollaborators(client collaboratorClient, orgName, repoName string,
12381215
actions[wantUser] = wantPermission
12391216

12401217
// Determine the appropriate action and log message
1241-
if currentCollaborators.has(normalizedUser) {
1218+
if _, exists := currentCollaborators.collaborators[normalizedUser]; exists {
12421219
logrus.Infof("Will update collaborator %s with %s permission", wantUser, wantPermission)
12431220
} else if pendingPermission, hasPendingInvitation := pendingInvitations[normalizedUser]; hasPendingInvitation {
12441221
logrus.Infof("Will update pending invitation for %s from %s to %s permission", wantUser, pendingPermission, wantPermission)
@@ -1249,9 +1226,9 @@ func configureCollaborators(client collaboratorClient, orgName, repoName string,
12491226

12501227
// Remove direct collaborators not in our config (including those with pending invitations)
12511228
// Since we only get direct collaborators via GraphQL, we can safely remove anyone not in config
1252-
for normalizedCurrentUser := range combinedCollaborators.permissions {
1229+
for normalizedCurrentUser := range combinedCollaborators.collaborators {
12531230
// Check if this user (normalized) is in our wanted config
1254-
if !wantedCollaborators.has(normalizedCurrentUser) {
1231+
if _, exists := wantedCollaborators.collaborators[normalizedCurrentUser]; !exists {
12551232
originalName := combinedCollaborators.originalName(normalizedCurrentUser)
12561233
actions[originalName] = github.None
12571234

@@ -1274,7 +1251,7 @@ func configureCollaborators(client collaboratorClient, orgName, repoName string,
12741251
normalizedUser := github.NormLogin(user)
12751252
if invitationID, hasPendingInvitation := pendingInvitationIDs[normalizedUser]; hasPendingInvitation {
12761253
// Use DeleteRepoInvitation (DELETE) for pending invitations with invitation ID
1277-
err = client.DeleteRepoInvitation(orgName, repoName, invitationID)
1254+
err = client.DeleteCollaboratorRepoInvitation(orgName, repoName, invitationID)
12781255
if err != nil {
12791256
logrus.WithError(err).Warnf("Failed to delete pending invitation for %s", user)
12801257
} else {
@@ -1294,7 +1271,7 @@ func configureCollaborators(client collaboratorClient, orgName, repoName string,
12941271
normalizedUser := github.NormLogin(user)
12951272
if invitationID, hasPendingInvitation := pendingInvitationIDs[normalizedUser]; hasPendingInvitation {
12961273
// Use UpdateRepoInvitation (PATCH) for pending invitations with invitation ID
1297-
err = client.UpdateRepoInvitation(orgName, repoName, invitationID, permission)
1274+
err = client.UpdateCollaboratorRepoInvitation(orgName, repoName, invitationID, permission)
12981275
if err != nil {
12991276
logrus.WithError(err).Warnf("Failed to update pending invitation for %s to %s permission", user, permission)
13001277
} else {

0 commit comments

Comments
 (0)