Reduce API calls made for single user events in GitHub Org Provider
Currently the provider will load all users group and all members of the groups when responding to an event on a single user. This can cause excessive API calls to page through group members when this information is just discarded in this instance. This change introduces single user versions of the code that will not page through all membership. Signed-off-by: Scott Guymer <scott.guymer@philips.com>
This commit is contained in:
@@ -0,0 +1,5 @@
|
||||
---
|
||||
'@backstage/plugin-catalog-backend-module-github': minor
|
||||
---
|
||||
|
||||
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.
|
||||
@@ -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 = {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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';
|
||||
|
||||
@@ -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, []);
|
||||
|
||||
@@ -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[]) {
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user