From 748810283361013a372dd4a67fa93cbbdf8afeec Mon Sep 17 00:00:00 2001
From: Carina Antunes <carina.oliveira.antunes@cern.ch>
Date: Wed, 21 Sep 2022 10:12:25 +0000
Subject: [PATCH] perf improvements
---
src/models/channel.ts | 9 ++++
.../impl/channels/add-group-to-channel.ts | 44 ++++++++++++-------
2 files changed, 36 insertions(+), 17 deletions(-)
diff --git a/src/models/channel.ts b/src/models/channel.ts
index cbe891c3..c929f713 100644
--- a/src/models/channel.ts
+++ b/src/models/channel.ts
@@ -405,6 +405,15 @@ export class Channel extends ApiKeyObject {
});
}
+ // Get QB Brackets channels by access: where group is Member
+ static qbGroupIsMember(qb: SelectQueryBuilder<unknown>, groupId: string): Brackets {
+ return new Brackets(accessQB => {
+ accessQB.where(':groupId IN (groups.id)', {
+ groupId: groupId,
+ });
+ });
+ }
+
async canSendCritical(authorizationBag: AuthorizationBag) {
if (!authorizationBag) return false;
if (!this.channelFlags?.includes(ChannelFlags.critical)) return false;
diff --git a/src/services/impl/channels/add-group-to-channel.ts b/src/services/impl/channels/add-group-to-channel.ts
index eae547a0..ae35e6bd 100644
--- a/src/services/impl/channels/add-group-to-channel.ts
+++ b/src/services/impl/channels/add-group-to-channel.ts
@@ -6,8 +6,6 @@ import { AuthorizationBag } from '../../../models/authorization-bag';
import { ServiceFactory } from '../../services-factory';
import { GroupsServiceInterface } from '../../groups-service';
import { AuditChannels } from '../../../log/auditing';
-import { ChannelResponse } from '../../../controllers/channels/dto';
-import { Group } from '../../../models/group';
import { GroupResponse } from '../../../controllers/channels/dto';
import { CernAuthorizationService } from '../../../models/cern-authorization-service';
@@ -16,28 +14,40 @@ export class AddGroupToChannel implements Command {
groupsService: GroupsServiceInterface = ServiceFactory.getGroupService();
async execute(transactionManager: EntityManager): Promise<GroupResponse> {
- const channel = await transactionManager.findOne(Channel, {
- relations: ['owner', 'adminGroup', 'groups'],
- where: {
- id: this.channelId,
- },
- });
-
+ const channel = await transactionManager.findOneBy(Channel, { id: this.channelId });
if (!channel) throw new NotFoundError('Channel does not exist');
- // you need to be channel admin
- if (!(await channel.isAdmin(this.authorizationBag)))
+ // Get current user groups, to be used next in SQL to compare with channel groups
+ const userGroups = await CernAuthorizationService.getCurrentUserGroups(this.authorizationBag.userName);
+
+ const qb = await transactionManager
+ .getRepository(Channel)
+ .createQueryBuilder('channel')
+ .leftJoin('channel.owner', 'owner')
+ .leftJoin('channel.adminGroup', 'adminGroup')
+ .leftJoin('channel.groups', 'groups')
+ .where('channel.id = :id', { id: this.channelId })
+ .select('channel.id');
+
+ let rawChannel = await qb
+ .clone()
+ .andWhere(Channel.qbUserIsAdmin(qb, this.authorizationBag.userId, userGroups))
+ .getRawOne();
+ if (!rawChannel) {
throw new ForbiddenError("You don't have the rights to manage this channel.");
+ }
- //if added group was already added
- if (channel.groups.filter(g => g.groupIdentifier === this.groupName).length > 0) {
+ const groupToAdd = await this.groupsService.GetOrCreateGroup(this.groupName);
+
+ rawChannel = await qb.clone().andWhere(Channel.qbGroupIsMember(qb, groupToAdd.id)).getRawOne();
+ if (rawChannel) {
throw new ForbiddenError(`Group ${this.groupName} is already a member.`);
}
- const groupToAdd = await this.groupsService.GetOrCreateGroup(this.groupName);
- channel.addGroup(groupToAdd);
- const updatedChannel = await transactionManager.save(channel);
- await AuditChannels.setValue(updatedChannel.id, {
+ // optimized save performance
+ await transactionManager.createQueryBuilder().relation(Channel, 'groups').of(channel).add(groupToAdd);
+
+ await AuditChannels.setValue(channel.id, {
event: 'AddGroup',
user: this.authorizationBag.email,
groupIdentifier: this.groupName,
--
GitLab