[permissions] Avoid N+1 on roles (#10041)

This commit is contained in:
Marie
2025-02-06 11:27:11 +01:00
committed by GitHub
parent 049a0118aa
commit 8a660d5d3f
5 changed files with 102 additions and 79 deletions

View File

@ -47,6 +47,6 @@ export class WorkspaceMember {
@Field(() => [RoleDTO], { nullable: true }) @Field(() => [RoleDTO], { nullable: true })
roles?: RoleDTO[]; roles?: RoleDTO[];
@Field(() => String) @Field(() => String, { nullable: true })
userWorkspaceId: string; userWorkspaceId?: string;
} }

View File

@ -13,7 +13,6 @@ import crypto from 'crypto';
import { GraphQLJSONObject } from 'graphql-type-json'; import { GraphQLJSONObject } from 'graphql-type-json';
import { FileUpload, GraphQLUpload } from 'graphql-upload'; import { FileUpload, GraphQLUpload } from 'graphql-upload';
import { isDefined } from 'twenty-shared';
import { In, Repository } from 'typeorm'; import { In, Repository } from 'typeorm';
import { SupportDriver } from 'src/engine/core-modules/environment/interfaces/support.interface'; import { SupportDriver } from 'src/engine/core-modules/environment/interfaces/support.interface';
@ -49,6 +48,7 @@ import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorat
import { OriginHeader } from 'src/engine/decorators/auth/origin-header.decorator'; import { OriginHeader } from 'src/engine/decorators/auth/origin-header.decorator';
import { DemoEnvGuard } from 'src/engine/guards/demo.env.guard'; import { DemoEnvGuard } from 'src/engine/guards/demo.env.guard';
import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard'; import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard';
import { RoleDTO } from 'src/engine/metadata-modules/role/dtos/role.dto';
import { UserRoleService } from 'src/engine/metadata-modules/user-role/user-role.service'; import { UserRoleService } from 'src/engine/metadata-modules/user-role/user-role.service';
import { AccountsToReconnectKeys } from 'src/modules/connected-account/types/accounts-to-reconnect-key-value.type'; import { AccountsToReconnectKeys } from 'src/modules/connected-account/types/accounts-to-reconnect-key-value.type';
import { streamToBuffer } from 'src/utils/stream-to-buffer'; import { streamToBuffer } from 'src/utils/stream-to-buffer';
@ -173,20 +173,35 @@ export class UserResolver {
const workspaceMembers: WorkspaceMember[] = []; const workspaceMembers: WorkspaceMember[] = [];
const userWorkspaces = await this.userWorkspaceRepository.find({ const permissionsEnabled = await this.featureFlagService.isFeatureEnabled(
where: { FeatureFlagKey.IsPermissionsEnabled,
userId: In(workspaceMemberEntities.map((entity) => entity.userId)), workspace.id,
workspaceId: workspace.id,
},
});
const userWorkspacesByUserId = new Map(
userWorkspaces.map((userWorkspace) => [
userWorkspace.userId,
userWorkspace,
]),
); );
let userWorkspacesByUserId = new Map<string, UserWorkspace>();
let rolesByUserWorkspaces = new Map<string, RoleDTO[]>();
if (permissionsEnabled === true) {
const userWorkspaces = await this.userWorkspaceRepository.find({
where: {
userId: In(workspaceMemberEntities.map((entity) => entity.userId)),
workspaceId: workspace.id,
},
});
userWorkspacesByUserId = new Map(
userWorkspaces.map((userWorkspace) => [
userWorkspace.userId,
userWorkspace,
]),
);
rolesByUserWorkspaces =
await this.userRoleService.getRolesByUserWorkspaces(
userWorkspaces.map((userWorkspace) => userWorkspace.id),
);
}
for (const workspaceMemberEntity of workspaceMemberEntities) { for (const workspaceMemberEntity of workspaceMemberEntities) {
if (workspaceMemberEntity.avatarUrl) { if (workspaceMemberEntity.avatarUrl) {
const avatarUrlToken = await this.fileService.encodeFileToken({ const avatarUrlToken = await this.fileService.encodeFileToken({
@ -197,45 +212,33 @@ export class UserResolver {
workspaceMemberEntity.avatarUrl = `${workspaceMemberEntity.avatarUrl}?token=${avatarUrlToken}`; workspaceMemberEntity.avatarUrl = `${workspaceMemberEntity.avatarUrl}?token=${avatarUrlToken}`;
} }
const userWorkspace = userWorkspacesByUserId.get( const workspaceMember = workspaceMemberEntity as WorkspaceMember;
workspaceMemberEntity.userId,
);
if (!userWorkspace) {
throw new Error('User workspace not found');
}
const permissionsEnabled = await this.featureFlagService.isFeatureEnabled(
FeatureFlagKey.IsPermissionsEnabled,
workspace.id,
);
const workspaceMember: WorkspaceMember = {
...workspaceMemberEntity,
userWorkspaceId: userWorkspace.id,
} as WorkspaceMember;
if (permissionsEnabled === true) { if (permissionsEnabled === true) {
const roles = await this.userRoleService const userWorkspace = userWorkspacesByUserId.get(
.getRolesForUserWorkspace(userWorkspace.id) workspaceMemberEntity.userId,
.then(([roleEntity]) => { );
if (!isDefined(roleEntity)) {
return [];
}
return [ if (!userWorkspace) {
{ throw new Error('User workspace not found');
id: roleEntity.id, }
label: roleEntity.label,
canUpdateAllSettings: roleEntity.canUpdateAllSettings,
description: roleEntity.description,
isEditable: roleEntity.isEditable,
userWorkspaceRoles: roleEntity.userWorkspaceRoles,
},
];
});
workspaceMember.roles = roles; workspaceMember.userWorkspaceId = userWorkspace.id;
const workspaceMemberRoles = (
rolesByUserWorkspaces.get(userWorkspace.id) ?? []
).map((roleEntity) => {
return {
id: roleEntity.id,
label: roleEntity.label,
canUpdateAllSettings: roleEntity.canUpdateAllSettings,
description: roleEntity.description,
isEditable: roleEntity.isEditable,
userWorkspaceRoles: roleEntity.userWorkspaceRoles,
};
});
workspaceMember.roles = workspaceMemberRoles;
} }
workspaceMembers.push(workspaceMember); workspaceMembers.push(workspaceMember);

View File

@ -21,8 +21,9 @@ export class PermissionsService {
}: { }: {
userWorkspaceId: string; userWorkspaceId: string;
}): Promise<Record<SettingsFeatures, boolean>> { }): Promise<Record<SettingsFeatures, boolean>> {
const [roleOfUserWorkspace] = const [roleOfUserWorkspace] = await this.userRoleService
await this.userRoleService.getRolesForUserWorkspace(userWorkspaceId); .getRolesByUserWorkspaces([userWorkspaceId])
.then((roles) => roles?.get(userWorkspaceId) ?? []);
let hasPermissionOnSettingFeature = false; let hasPermissionOnSettingFeature = false;
@ -46,10 +47,11 @@ export class PermissionsService {
userWorkspaceId: string; userWorkspaceId: string;
setting: SettingsFeatures; setting: SettingsFeatures;
}): Promise<void> { }): Promise<void> {
const [userWorkspaceRole] = const [roleOfUserWorkspace] = await this.userRoleService
await this.userRoleService.getRolesForUserWorkspace(userWorkspaceId); .getRolesByUserWorkspaces([userWorkspaceId])
.then((roles) => roles?.get(userWorkspaceId) ?? []);
if (userWorkspaceRole?.canUpdateAllSettings === true) { if (roleOfUserWorkspace?.canUpdateAllSettings === true) {
return; return;
} }

View File

@ -101,9 +101,12 @@ export class RoleResolver {
}); });
} }
const roles = await this.userRoleService.getRolesForUserWorkspace( const roles = await this.userRoleService
userWorkspace.id, .getRolesByUserWorkspaces([userWorkspace.id])
); .then(
(rolesByUserWorkspaces) =>
rolesByUserWorkspaces?.get(userWorkspace.id) ?? [],
);
return { return {
...workspaceMember, ...workspaceMember,

View File

@ -60,7 +60,9 @@ export class UserRoleService {
); );
} }
const [currentRole] = await this.getRolesForUserWorkspace(userWorkspace.id); const roles = await this.getRolesByUserWorkspaces([userWorkspace.id]);
const currentRole = roles.get(userWorkspace.id)?.[0];
if (currentRole?.id === roleId) { if (currentRole?.id === roleId) {
return; return;
@ -96,33 +98,42 @@ export class UserRoleService {
}); });
} }
public async getRolesForUserWorkspace( public async getRolesByUserWorkspaces(
userWorkspaceId: string, userWorkspaceIds: string[],
): Promise<RoleDTO[] | []> { ): Promise<Map<string, RoleDTO[]>> {
const userWorkspaceRole = await this.userWorkspaceRoleRepository.findOne({ if (!userWorkspaceIds.length) {
where: { return new Map();
userWorkspaceId,
},
});
if (!isDefined(userWorkspaceRole)) {
return [];
} }
const role = await this.roleRepository.findOne({ const allUserWorkspaceRoles = await this.userWorkspaceRoleRepository.find({
where: { where: {
id: userWorkspaceRole.roleId, userWorkspaceId: In(userWorkspaceIds),
},
relations: {
role: true,
}, },
}); });
if (!isDefined(role)) { if (!allUserWorkspaceRoles.length) {
throw new PermissionsException( return new Map();
'Role not found', }
PermissionsExceptionCode.ROLE_NOT_FOUND,
const rolesMap = new Map<string, RoleDTO[]>();
for (const userWorkspaceId of userWorkspaceIds) {
const userWorkspaceRolesOfUserWorkspace = allUserWorkspaceRoles.filter(
(userWorkspaceRole) =>
userWorkspaceRole.userWorkspaceId === userWorkspaceId,
); );
const rolesOfUserWorkspace = userWorkspaceRolesOfUserWorkspace
.map((userWorkspaceRole) => userWorkspaceRole.role)
.filter(isDefined);
rolesMap.set(userWorkspaceId, rolesOfUserWorkspace);
} }
return [role]; return rolesMap;
} }
public async getWorkspaceMembersAssignedToRole( public async getWorkspaceMembersAssignedToRole(
@ -169,9 +180,13 @@ export class UserRoleService {
userWorkspaceId: string, userWorkspaceId: string,
workspaceId: string, workspaceId: string,
): Promise<void> { ): Promise<void> {
const roles = await this.getRolesForUserWorkspace(userWorkspaceId); const roles = await this.getRolesByUserWorkspaces([userWorkspaceId]);
const adminRole = roles.find((role: RoleDTO) => role.isEditable === false); const currentRoles = roles.get(userWorkspaceId);
const adminRole = currentRoles?.find(
(role: RoleDTO) => role.isEditable === false,
);
if (isDefined(adminRole)) { if (isDefined(adminRole)) {
const workspaceMembersWithAdminRole = const workspaceMembersWithAdminRole =