Merge pull request #30498 from philips-forks/scott/fix_provider

Reduce API calls made for single user events in GitHub Org Provider

.changeset/ripe-comics-sip.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-backend-module-github': patch
+---
+
+This change introduces single user versions of the user group resolution code in the multi org provider that will not page through all membership when updating a single user.

plugins/catalog-backend-module-github/src/lib/github.test.ts

@@ -36,6 +36,7 @@ import {
   createRemoveEntitiesOperation,
   createReplaceEntitiesOperation,
   createGraphqlClient,
+  getOrganizationTeamsForUser,
 } from './github';
 import { Octokit } from '@octokit/core';
 import { throttling } from '@octokit/plugin-throttling';
@@ -53,6 +54,79 @@ describe('github', () => {
 
   const graphql = graphqlOctokit.defaults({});
 
+  describe('getOrganizationTeamsForUser', () => {
+    const org = 'my-org';
+    const userLogin = 'testuser';
+
+    it('returns teams for a user', async () => {
+      server.use(
+        graphqlMsw.query('teams', () =>
+          HttpResponse.json({
+            data: {
+              organization: {
+                teams: {
+                  pageInfo: { hasNextPage: false, endCursor: null },
+                  nodes: [
+                    {
+                      slug: 'team1',
+                      combinedSlug: 'my-org/team1',
+                      name: 'Team 1',
+                      description: 'desc',
+                      avatarUrl: '',
+                      editTeamUrl: '',
+                      parentTeam: null,
+                    },
+                  ],
+                },
+              },
+            },
+          }),
+        ),
+      );
+
+      const mockTransformer = jest.fn().mockImplementation(async team => ({
+        kind: 'Group',
+        metadata: { name: team.slug },
+      }));
+
+      const { teams } = await getOrganizationTeamsForUser(
+        graphql as any,
+        org,
+        userLogin,
+        mockTransformer as any,
+      );
+      expect(Array.isArray(teams)).toBe(true);
+      expect(teams[0]).toEqual({ kind: 'Group', metadata: { name: 'team1' } });
+      expect(mockTransformer).toHaveBeenCalled();
+    });
+
+    it('returns an empty array if no teams found', async () => {
+      server.use(
+        graphqlMsw.query('teams', () =>
+          HttpResponse.json({
+            data: {
+              organization: {
+                teams: {
+                  pageInfo: { hasNextPage: false, endCursor: null },
+                  nodes: [],
+                },
+              },
+            },
+          }),
+        ),
+      );
+      const mockTransformer = jest.fn().mockResolvedValue(undefined);
+      const { teams } = await getOrganizationTeamsForUser(
+        graphql as any,
+        org,
+        userLogin,
+        mockTransformer as any,
+      );
+      expect(Array.isArray(teams)).toBe(true);
+      expect(teams.length).toBe(0);
+    });
+  });
+
   describe('getOrganizationUsers using defaultUserMapper', () => {
     it('reads members', async () => {
       const input: QueryResponse = {

plugins/catalog-backend-module-github/src/lib/github.ts

@@ -346,6 +346,59 @@ export async function getOrganizationTeamsFromUsers(
   return { teams };
 }
 
+export async function getOrganizationTeamsForUser(
+  client: typeof graphql,
+  org: string,
+  userLogin: string,
+  teamTransformer: TeamTransformer,
+): Promise<{ teams: Entity[] }> {
+  const query = `
+   query teams($org: String!, $cursor: String, $userLogins: [String!] = "") {
+  organization(login: $org) {
+    teams(first: 100, after: $cursor, userLogins: $userLogins) {
+      pageInfo {
+        hasNextPage
+        endCursor
+      }
+      nodes {
+        slug
+        combinedSlug
+        name
+        description
+        avatarUrl
+        editTeamUrl
+        parentTeam {
+          slug
+        }
+      }
+    }
+  }
+}`;
+
+  const materialisedTeams = async (
+    item: GithubTeamResponse,
+    ctx: TransformerContext,
+  ): Promise<Entity | undefined> => {
+    const team: GithubTeam = {
+      ...item,
+      members: [{ login: userLogin }],
+    };
+
+    return await teamTransformer(team, ctx);
+  };
+
+  const teams = await queryWithPaging(
+    client,
+    query,
+    org,
+    r => r.organization?.teams,
+    materialisedTeams,
+    { org, userLogins: [userLogin] },
+  );
+
+  return { teams };
+}
+
 export async function getOrganizationsFromUser(
   client: typeof graphql,
   user: string,

plugins/catalog-backend-module-github/src/lib/index.ts

@@ -30,5 +30,9 @@ export {
   defaultOrganizationTeamTransformer,
   type TransformerContext,
 } from './defaultTransformers';
-export { assignGroupsToUsers, buildOrgHierarchy } from './org';
+export {
+  assignGroupsToUser,
+  assignGroupsToUsers,
+  buildOrgHierarchy,
+} from './org';
 export { parseGithubOrgUrl } from './util';

plugins/catalog-backend-module-github/src/lib/org.test.ts

@@ -15,7 +15,12 @@
  */
 
 import { GroupEntity, UserEntity } from '@backstage/catalog-model';
-import { assignGroupsToUsers, buildMemberOf, buildOrgHierarchy } from './org';
+import {
+  assignGroupsToUsers,
+  assignGroupsToUser,
+  buildMemberOf,
+  buildOrgHierarchy,
+} from './org';
 
 function u(name: string, memberOf: string[] = []): UserEntity {
   return {
@@ -115,6 +120,42 @@ describe('assignGroupsToUsers', () => {
   });
 });
 
+describe('assignGroupsToUser', () => {
+  it('should assign the correct groups to a user', () => {
+    const user: UserEntity = u('u1');
+    const groups: GroupEntity[] = [
+      {
+        apiVersion: 'backstage.io/v1alpha1',
+        kind: 'Group',
+        metadata: { name: 'g1' },
+        spec: { type: 'team', children: [], members: ['default/u1'] },
+      },
+      {
+        apiVersion: 'backstage.io/v1alpha1',
+        kind: 'Group',
+        metadata: { name: 'g2' },
+        spec: { type: 'team', children: [], members: ['u2'] },
+      },
+    ];
+    assignGroupsToUser(user, groups);
+    expect(user.spec.memberOf).toEqual(['g1']);
+  });
+
+  it('should not assign any groups if user is not a member', () => {
+    const user: UserEntity = u('u3');
+    const groups: GroupEntity[] = [
+      {
+        apiVersion: 'backstage.io/v1alpha1',
+        kind: 'Group',
+        metadata: { name: 'g1' },
+        spec: { type: 'team', children: [], members: ['u1'] },
+      },
+    ];
+    assignGroupsToUser(user, groups);
+    expect(user.spec.memberOf).toEqual([]);
+  });
+});
+
 describe('buildMemberOf', () => {
   it('fills indirect member of groups', () => {
     const a = g('a', undefined, []);

plugins/catalog-backend-module-github/src/lib/org.ts

@@ -91,6 +91,29 @@ export function assignGroupsToUsers(
   }
 }
 
+// Assign all relevant groups to a single user if the user is a member of each group.
+export function assignGroupsToUser(user: UserEntity, groups: GroupEntity[]) {
+  const userRef = stringifyEntityRef(user);
+  for (const group of groups) {
+    const groupKey =
+      group.metadata.namespace && group.metadata.namespace !== DEFAULT_NAMESPACE
+        ? `${group.metadata.namespace}/${group.metadata.name}`
+        : group.metadata.name;
+    const memberRefs =
+      group.spec.members?.map(m =>
+        stringifyEntityRef(parseEntityRef(m, { defaultKind: 'user' })),
+      ) || [];
+    if (memberRefs.includes(userRef)) {
+      if (!user.spec.memberOf) {
+        user.spec.memberOf = [];
+      }
+      if (!user.spec.memberOf.includes(groupKey)) {
+        user.spec.memberOf.push(groupKey);
+      }
+    }
+  }
+}
+
 // Ensure that users have their transitive group memberships. Requires that
 // the groups were previously processed with buildOrgHierarchy()
 export function buildMemberOf(groups: GroupEntity[], users: UserEntity[]) {

plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.test.ts

@@ -2306,23 +2306,7 @@ describe('GithubMultiOrgEntityProvider', () => {
             organization: {
               teams: {
                 pageInfo: { hasNextPage: false },
-                nodes: [
-                  {
-                    slug: 'team',
-                    combinedSlug: 'orgA/team',
-                    name: 'TeamA',
-                    description: 'The one and only team',
-                    avatarUrl: 'http://example.com/team.jpeg',
-                    editTeamUrl: 'https://example.com',
-                    parentTeam: {
-                      slug: 'parent',
-                    },
-                    members: {
-                      pageInfo: { hasNextPage: false },
-                      nodes: [],
-                    },
-                  },
-                ],
+                nodes: [],
               },
             },
           })

plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts

@@ -60,6 +60,7 @@ import {
   defaultOrganizationTeamTransformer,
   defaultUserTransformer,
   getOrganizationTeams,
+  assignGroupsToUser,
   getOrganizationUsers,
   GithubTeam,
   TeamTransformer,
@@ -73,6 +74,7 @@ import {
 import {
   getOrganizationsFromUser,
   getOrganizationTeam,
+  getOrganizationTeamsForUser,
   getOrganizationTeamsFromUsers,
 } from '../lib/github';
 import { splitTeamSlug } from '../lib/util';
@@ -556,15 +558,15 @@ export class GithubMultiOrgEntityProvider implements EntityProvider {
           headers: orgHeaders,
         });
 
-        const { teams } = await getOrganizationTeamsFromUsers(
+        const { teams } = await getOrganizationTeamsForUser(
           orgClient,
           userOrg,
-          [login],
+          login,
           this.defaultMultiOrgTeamTransformer.bind(this),
         );
 
         if (isUserEntity(user) && areGroupEntities(teams)) {
-          assignGroupsToUsers([user], teams);
+          assignGroupsToUser(user, teams);
         }
       }
     }
@@ -799,15 +801,15 @@ export class GithubMultiOrgEntityProvider implements EntityProvider {
           headers: orgHeaders,
         });
 
-        const { teams } = await getOrganizationTeamsFromUsers(
+        const { teams } = await getOrganizationTeamsForUser(
           orgClient,
           userOrg,
-          [login],
+          login,
           this.defaultMultiOrgTeamTransformer.bind(this),
         );
 
         if (areGroupEntities(teams)) {
-          assignGroupsToUsers([user], teams);
+          assignGroupsToUser(user, teams);
         }
       }