⏩ check membership as needed (#84)

* General org membership checking is being removed and replaced by just
in time checks to save on the amount of calls going to GitHub's API,
which will help both general sync performance (as now there is no
up-front need to fetch all members, which for large orgs is intense) as
well as with issues where the sync bot was running into GitHub API rate
limits (less calls == less chance of hitting the API rate limit)

Author: JoshuaTheMiller

src/handlers/syncSpecificTeamHandler.ts

@@ -36,12 +36,6 @@ export async function syncSpecificTeamHandler(
 
     const invitationsClient = GetInvitationsClient(orgClient);
 
-    const existingOrgMembers = await orgClient.GetOrgMembers();
-
-    if(!existingOrgMembers.successful) {
-        return res.status(500).json("Unable to fetch org members");    
-    }
-
     const invitesResponse = await invitationsClient.ListInvites();
 
     if(!invitesResponse.successful) {
@@ -60,7 +54,7 @@ export async function syncSpecificTeamHandler(
 
     const sourceTeamMap = orgConfig.data.DisplayNameToSourceMap;
 
-    const response = await SyncTeam(teamName, orgClient, appConfig, existingOrgMembers.data, invites, sourceTeamMap);
+    const response = await SyncTeam(teamName, orgClient, appConfig, invites, sourceTeamMap);
 
     return res.status(200).json(response);
 }

src/services/gitHub.ts

@@ -364,19 +364,6 @@ class InstalledGitHubClient implements InstalledClient {
         }
     }
 
-    public async GetOrgMembers(): Response<GitHubId[]> {
-        const response = await this.gitHubClient.paginate(this.gitHubClient.rest.orgs.listMembers, {
-            org: this.orgName
-        })
-
-        return {
-            successful: true,
-            data: response.map(i => {
-                return i.login
-            })
-        }
-    }
-
     public GetCurrentOrgName(): string {
         return this.orgName;
     }

src/services/gitHubCache.ts

@@ -148,9 +148,5 @@ export class GitHubClientCache implements InstalledClient {
 
     GetConfigurationForInstallation(): Response<OrgConfig> {
         return this.client.GetConfigurationForInstallation();
-    }
-
-    GetOrgMembers(): Response<string[]> {
-        return this.client.GetOrgMembers();
-    }
+    }    
 }

src/services/gitHubTypes.ts

@@ -32,8 +32,7 @@ export interface InstalledClient {
     RemoveTeamMemberAsync(team: GitHubTeamName, user: GitHubId): RemoveMemberResponse
     UpdateTeamDetails(team: GitHubTeamName, description: string): Response
     AddSecurityManagerTeam(team: GitHubTeamName): Promise<unknown>
-    GetConfigurationForInstallation(): Response<OrgConfig>    
-    GetOrgMembers(): Response<GitHubId[]>
+    GetConfigurationForInstallation(): Response<OrgConfig>        
     SetOrgRole(id: GitHubId, role: OrgRoles): Response
     GetPendingOrgInvites():Response<OrgInvite[]>
     CancelOrgInvite(invite:OrgInvite): Response    

src/services/githubSync.ts

@@ -86,7 +86,7 @@ async function SynchronizeOrgMembers(installedGitHubClient: InstalledClient, tea
     };
 }
 
-async function SynchronizeGitHubTeam(installedGitHubClient: InstalledClient, teamName: string, config: AppConfig, existingMembers: GitHubId[], existingInvites: OrgInvite[], sourceTeamMap: Map<string, string>, checkOrgMembers: boolean = true) {
+async function SynchronizeGitHubTeam(installedGitHubClient: InstalledClient, teamName: string, config: AppConfig, existingInvites: OrgInvite[], sourceTeamMap: Map<string, string>, checkOrgMembers: boolean = true) {
     function GetSourceOrReturn(teamName: string) {
         return sourceTeamMap.get(teamName) ?? teamName;
     }
@@ -110,10 +110,16 @@ async function SynchronizeGitHubTeam(installedGitHubClient: InstalledClient, tea
 
     const orgName = installedGitHubClient.GetCurrentOrgName();
 
+    const memberCheckFunc = async (id:GitHubId) => {
+        const response = await installedGitHubClient.IsUserMember(id);
+
+        return response.successful && response.data;
+    }
+
     const validMemberCheckResults = await Promise.all(trueMembersList.map(tm => checkValidOrgMember({
         gitHubId: tm,
         checkOrgMembers: checkOrgMembers,
-        existingMembers: existingMembers,
+        isExistingMember: memberCheckFunc,
         idsWithInvites: idsWithInvites,
         installedGitHubClient: installedGitHubClient,
         orgName: orgName
@@ -210,7 +216,7 @@ async function SyncSecurityManagers(opts: {
         }
 
 
-        await SynchronizeGitHubTeam(installedGitHubClient, t, appConfig, orgMembers.OrgMembers, currentInvites, displayNameToSourceMap);
+        await SynchronizeGitHubTeam(installedGitHubClient, t, appConfig, currentInvites, displayNameToSourceMap);
 
         Log(`Add Security Manager Team for ${installedGitHubClient.GetCurrentOrgName()}: ${t}`)
         const addResult = await installedGitHubClient.AddSecurityManagerTeam(t);
@@ -352,17 +358,7 @@ async function syncOrg(installedGitHubClient: InstalledClient, appConfig: AppCon
 
         currentMembers = currentMembersResponse.OrgMembers;
 
-        await SynchronizeGitHubTeam(installedGitHubClient, membersGroupName, appConfig, currentMembers, currentInvites, orgConfig.DisplayNameToSourceMap);
-    }
-
-    if (currentMembers.length == 0) {
-        const getOrgMembersResponse = await installedGitHubClient.GetOrgMembers();
-
-        if (!getOrgMembersResponse.successful) {
-            throw Error("Unable to get current org members");
-        }
-
-        currentMembers = getOrgMembersResponse.data;
+        await SynchronizeGitHubTeam(installedGitHubClient, membersGroupName, appConfig, currentInvites, orgConfig.DisplayNameToSourceMap);
     }
 
     if (!gitHubTeams || gitHubTeams.length < 1) {
@@ -372,7 +368,7 @@ async function syncOrg(installedGitHubClient: InstalledClient, appConfig: AppCon
 
     async function syncTeam(teamName: string, orgConfig: OrgConfig) {
         Log(`Syncing Team Members for ${teamName} in ${installedGitHubClient.GetCurrentOrgName()}`)
-        await SynchronizeGitHubTeam(installedGitHubClient, teamName, appConfig, currentMembers, currentInvites, orgConfig.DisplayNameToSourceMap);
+        await SynchronizeGitHubTeam(installedGitHubClient, teamName, appConfig, currentInvites, orgConfig.DisplayNameToSourceMap);
     }
 
     const teamSyncPromises = gitHubTeams.map(t => syncTeam(t, orgConfig));
@@ -414,8 +410,8 @@ async function syncOrg(installedGitHubClient: InstalledClient, appConfig: AppCon
 }
 
 
-export async function SyncTeam(teamName: string, client: InstalledClient, config: AppConfig, existingMembers: GitHubId[], invites: OrgInvite[], sourceTeamMap: Map<string, string>) {
-    const response = await SynchronizeGitHubTeam(client, teamName, config, existingMembers, invites, sourceTeamMap, true);
+export async function SyncTeam(teamName: string, client: InstalledClient, config: AppConfig, invites: OrgInvite[], sourceTeamMap: Map<string, string>) {
+    const response = await SynchronizeGitHubTeam(client, teamName, config, invites, sourceTeamMap, true);
 
     return response;
 }
@@ -517,9 +513,9 @@ async function checkValidOrgMember(opts: {
     checkOrgMembers: boolean,
     orgName: string,
     idsWithInvites: Set<string>,
-    existingMembers: GitHubId[]
+    isExistingMember: (id:GitHubId) => Promise<boolean>
 }) {
-    const { installedGitHubClient, gitHubId, checkOrgMembers, orgName, idsWithInvites, existingMembers } = opts;
+    const { installedGitHubClient, gitHubId, checkOrgMembers, orgName, idsWithInvites, isExistingMember } = opts;
 
     const isUserReal = await installedGitHubClient.DoesUserExist(gitHubId);
 
@@ -538,9 +534,9 @@ async function checkValidOrgMember(opts: {
             message: `User '${gitHubId} has a Pending Invite to ${orgName}`
         };
     }
-
-    if (checkOrgMembers) {
-        const isMember = existingMembers.filter(em => em == gitHubId);
+    
+    if (checkOrgMembers) {        
+        const isMember = await isExistingMember(gitHubId);        
 
         if (!isMember) {
             return {