Skip to content

Commit b2e4fad

Browse files
committed
[server] Optimize getTeamMembers (N queries → 1 query)
1 parent 9079cd5 commit b2e4fad

File tree

7 files changed

+43
-25
lines changed

7 files changed

+43
-25
lines changed

components/gitpod-db/src/team-db.spec.db.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { TypeORM } from './typeorm/typeorm';
1515
import { DBTeam } from './typeorm/entity/db-team';
1616
import { DBTeamMembership } from './typeorm/entity/db-team-membership';
1717
import { DBUser } from './typeorm/entity/db-user';
18+
import { DBIdentity } from './typeorm/entity/db-identity';
1819

1920
@suite class TeamDBSpec {
2021

@@ -35,6 +36,7 @@ import { DBUser } from './typeorm/entity/db-user';
3536
await manager.getRepository(DBTeam).delete({});
3637
await manager.getRepository(DBTeamMembership).delete({});
3738
await manager.getRepository(DBUser).delete({});
39+
await manager.getRepository(DBIdentity).delete({});
3840
}
3941

4042
@test(timeout(10000))
@@ -48,5 +50,18 @@ import { DBUser } from './typeorm/entity/db-user';
4850
expect(dbResult[0].name).to.eq('Ground Control');
4951
}
5052

53+
@test(timeout(10000))
54+
public async findTeamMembers() {
55+
const user = await this.userDb.newUser();
56+
user.identities.push({ authProviderId: 'GitHub', authId: '1234', authName: 'Major Tom', primaryEmail: '[email protected]' });
57+
await this.userDb.storeUser(user);
58+
const team = await this.db.createTeam(user.id, 'Flight Crew');
59+
const members = await this.db.findMembersByTeam(team.id);
60+
expect(members.length).to.eq(1);
61+
expect(members[0].userId).to.eq(user.id);
62+
expect(members[0].primaryEmail).to.eq('[email protected]');
63+
}
64+
5165
}
66+
5267
module.exports = new TeamDBSpec()

components/gitpod-db/src/team-db.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,12 @@
44
* See License.enterprise.txt in the project root folder.
55
*/
66

7-
import { Team } from "@gitpod/gitpod-protocol";
8-
import { DBTeamMembership } from "./typeorm/entity/db-team-membership";
7+
import { Team, TeamMemberInfo } from "@gitpod/gitpod-protocol";
98

109
export const TeamDB = Symbol('TeamDB');
1110
export interface TeamDB {
1211
findTeamById(teamId: string): Promise<Team | undefined>;
13-
findMembershipsByTeam(teamId: string): Promise<DBTeamMembership[]>;
12+
findMembersByTeam(teamId: string): Promise<TeamMemberInfo[]>;
1413
findTeamsByUser(userId: string): Promise<Team[]>;
1514
createTeam(userId: string, name: string): Promise<Team>;
1615
}

components/gitpod-db/src/typeorm/team-db-impl.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,15 @@
44
* See License-AGPL.txt in the project root for license information.
55
*/
66

7+
import { Team, TeamMemberInfo, User } from "@gitpod/gitpod-protocol";
78
import { inject, injectable } from "inversify";
89
import { TypeORM } from "./typeorm";
910
import { Repository } from "typeorm";
1011
import * as uuidv4 from 'uuid/v4';
1112
import { TeamDB } from "../team-db";
1213
import { DBTeam } from "./entity/db-team";
1314
import { DBTeamMembership } from "./entity/db-team-membership";
14-
import { Team } from "@gitpod/gitpod-protocol";
15+
import { DBUser } from "./entity/db-user";
1516

1617
@injectable()
1718
export class TeamDBImpl implements TeamDB {
@@ -29,14 +30,27 @@ export class TeamDBImpl implements TeamDB {
2930
return (await this.getEntityManager()).getRepository<DBTeamMembership>(DBTeamMembership);
3031
}
3132

33+
protected async getUserRepo(): Promise<Repository<DBUser>> {
34+
return (await this.getEntityManager()).getRepository<DBUser>(DBUser);
35+
}
36+
3237
public async findTeamById(teamId: string): Promise<Team | undefined> {
3338
const teamRepo = await this.getTeamRepo();
3439
return teamRepo.findOne({ id: teamId });
3540
}
3641

37-
public async findMembershipsByTeam(teamId: string): Promise<DBTeamMembership[]> {
42+
public async findMembersByTeam(teamId: string): Promise<TeamMemberInfo[]> {
3843
const membershipRepo = await this.getMembershipRepo();
39-
return membershipRepo.find({ teamId });
44+
const userRepo = await this.getUserRepo();
45+
const memberships = await membershipRepo.find({ teamId });
46+
const users = await userRepo.findByIds(memberships.map(m => m.userId));
47+
return users.map(u => ({
48+
userId: u.id,
49+
fullName: u.fullName || u.name,
50+
primaryEmail: User.getPrimaryEmail(u),
51+
avatarUrl: u.avatarUrl,
52+
memberSince: u.creationDate,
53+
}));
4054
}
4155

4256
public async findTeamsByUser(userId: string): Promise<Team[]> {

components/gitpod-db/src/typeorm/user-db-impl.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,8 @@ export class TypeORMUserDBImpl implements UserDB {
205205
}
206206

207207
public async findIdentitiesByName(identity: Identity): Promise<Identity[]> {
208-
const userRepo = await this.getIdentitiesRepo();
209-
const qBuilder = userRepo.createQueryBuilder('identity')
208+
const repo = await this.getIdentitiesRepo();
209+
const qBuilder = repo.createQueryBuilder('identity')
210210
.where(`identity.authProviderId = :authProviderId`, { authProviderId: identity.authProviderId })
211211
.andWhere(`identity.deleted != true`)
212212
.andWhere(`identity.authName = :authName`, { authName: identity.authName });

components/gitpod-db/src/user-db.spec.db.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import { DBWorkspaceInstance } from './typeorm/entity/db-workspace-instance';
3636
const typeorm = testContainer.get<TypeORM>(TypeORM);
3737
const mnr = await typeorm.getConnection();
3838
await mnr.getRepository(DBUser).delete({});
39+
await mnr.getRepository(DBIdentity).delete({});
3940
await mnr.getRepository(DBWorkspace).delete({});
4041
await mnr.getRepository(DBWorkspaceInstance).delete({});
4142
}

components/server/src/auth/resource-access.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@
44
* See License-AGPL.txt in the project root for license information.
55
*/
66

7-
import { GitpodToken, Snapshot, Team, Token, User, UserEnvVar, Workspace, WorkspaceInstance } from "@gitpod/gitpod-protocol";
8-
import { DBTeamMembership } from '@gitpod/gitpod-db/lib/typeorm/entity/db-team-membership';
7+
import { GitpodToken, Snapshot, Team, TeamMemberInfo, Token, User, UserEnvVar, Workspace, WorkspaceInstance } from "@gitpod/gitpod-protocol";
98

109
declare var resourceInstance: GuardedResource;
1110
export type GuardedResourceKind = typeof resourceInstance.kind;
@@ -83,7 +82,7 @@ export interface GuardEnvVar {
8382
export interface GuardedTeam {
8483
kind: "team";
8584
subject: Team;
86-
memberships: DBTeamMembership[];
85+
members: TeamMemberInfo[];
8786
}
8887

8988
export interface GuardedGitpodToken {
@@ -157,7 +156,7 @@ export class OwnerResourceGuard implements ResourceAccessGuard {
157156
case "envVar":
158157
return resource.subject.userId === this.userId;
159158
case "team":
160-
return resource.memberships.some(membership => membership.userId === this.userId);
159+
return resource.members.some(m => m.userId === this.userId);
161160
}
162161
}
163162

components/server/src/workspace/gitpod-server-impl.ts

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ import { MessageBusIntegration } from './messagebus-integration';
4646
import { WorkspaceDeletionService } from './workspace-deletion-service';
4747
import { WorkspaceFactory } from './workspace-factory';
4848
import { WorkspaceStarter } from './workspace-starter';
49-
import { DBTeamMembership } from "@gitpod/gitpod-db/lib/typeorm/entity/db-team-membership";
5049

5150

5251
@injectable()
@@ -1396,18 +1395,9 @@ export class GitpodServerImpl<Client extends GitpodClient, Server extends Gitpod
13961395
if (!team) {
13971396
throw new ResponseError(ErrorCodes.NOT_FOUND, "Team not found");
13981397
}
1399-
const memberships = await this.teamDB.findMembershipsByTeam(team.id);
1400-
await this.guardAccess({ kind: "team", subject: team, memberships }, "get");
1401-
return Promise.all(memberships.map(async (m: DBTeamMembership): Promise<TeamMemberInfo> => {
1402-
const member = await this.userDB.findUserById(m.userId);
1403-
return {
1404-
userId: m.userId,
1405-
fullName: member?.fullName || member?.name,
1406-
primaryEmail: !!member ? User.getPrimaryEmail(member) : undefined,
1407-
avatarUrl: member?.avatarUrl,
1408-
memberSince: m.creationTime,
1409-
};
1410-
}));
1398+
const members = await this.teamDB.findMembersByTeam(team.id);
1399+
await this.guardAccess({ kind: "team", subject: team, members }, "get");
1400+
return members;
14111401
}
14121402

14131403
public async createTeam(name: string): Promise<Team> {

0 commit comments

Comments
 (0)