rework bugfix to be more performant and compatible with other app parts

Author: ArtemHoruzhenko

redisinsight/api/src/modules/certificate/ca-certificate.controller.ts

@@ -14,8 +14,6 @@ import {
 } from '@nestjs/swagger';
 import { CaCertificate } from 'src/modules/certificate/models/ca-certificate';
 import { CaCertificateService } from 'src/modules/certificate/ca-certificate.service';
-import { RequestSessionMetadata } from 'src/common/decorators';
-import { SessionMetadata } from 'src/common/models';
 
 @ApiTags('TLS Certificates')
 @Controller('certificates/ca')
@@ -41,9 +39,8 @@ export class CaCertificateController {
   @ApiOperation({ description: 'Delete Ca Certificate by id' })
   @ApiParam({ name: 'id', type: String })
   async delete(
-    @RequestSessionMetadata() sessionMetadata: SessionMetadata,
-      @Param('id') id: string,
+    @Param('id') id: string,
   ): Promise<void> {
-    await this.service.delete(sessionMetadata, id);
+    await this.service.delete(id);
   }
 }

redisinsight/api/src/modules/certificate/ca-certificate.service.ts

@@ -12,8 +12,6 @@ import { CaCertificateRepository } from 'src/modules/certificate/repositories/ca
 import { CaCertificate } from 'src/modules/certificate/models/ca-certificate';
 import { CreateCaCertificateDto } from 'src/modules/certificate/dto/create.ca-certificate.dto';
 import { classToClass } from 'src/utils';
-import { SessionMetadata } from 'src/common/models';
-import { DatabaseRepository } from 'src/modules/database/repositories/database.repository';
 import { RedisClientStorage } from 'src/modules/redis/redis.client.storage';
 
 @Injectable()
@@ -22,7 +20,6 @@ export class CaCertificateService {
 
   constructor(
     private readonly repository: CaCertificateRepository,
-    private readonly databaseRepository: DatabaseRepository,
     private redisClientStorage: RedisClientStorage,
   ) {}
 
@@ -60,17 +57,14 @@ export class CaCertificateService {
     }
   }
 
-  async delete(sessionMetadata: SessionMetadata, id: string): Promise<void> {
+  async delete(id: string): Promise<void> {
     try {
-      const databases = await this.databaseRepository.list(sessionMetadata);
-      await Promise.all(databases.map(async (database) => {
-        const db = await this.databaseRepository.get(sessionMetadata, database.id);
-        if (db.caCert?.id === id) {
-          // If the certificate is used by the database, remove the client
-          await this.redisClientStorage.removeManyByMetadata({ databaseId: db.id });
-        }
+      const { affectedDatabases } = await this.repository.delete(id);
+
+      await Promise.all(affectedDatabases.map(async (databaseId) => {
+        // If the certificate is used by the database, remove the client
+        await this.redisClientStorage.removeManyByMetadata({ databaseId });
       }));
-      await this.repository.delete(id);
     } catch (error) {
       this.logger.error(`Failed to delete certificate ${id}`, error);
 

redisinsight/api/src/modules/certificate/client-certificate.controller.ts

@@ -12,8 +12,6 @@ import {
   ApiParam,
   ApiTags,
 } from '@nestjs/swagger';
-import { RequestSessionMetadata } from 'src/common/decorators';
-import { SessionMetadata } from 'src/common/models';
 import { ClientCertificateService } from 'src/modules/certificate/client-certificate.service';
 import { ClientCertificate } from 'src/modules/certificate/models/client-certificate';
 
@@ -42,9 +40,8 @@ export class ClientCertificateController {
   @ApiOperation({ description: 'Delete Client Certificate pair by id' })
   @ApiParam({ name: 'id', type: String })
   async deleteClientCertificatePair(
-    @RequestSessionMetadata() sessionMetadata: SessionMetadata,
-      @Param('id') id: string,
+    @Param('id') id: string,
   ): Promise<void> {
-    await this.service.delete(sessionMetadata, id);
+    await this.service.delete(id);
   }
 }

redisinsight/api/src/modules/certificate/client-certificate.service.ts

@@ -12,8 +12,6 @@ import { ClientCertificateRepository } from 'src/modules/certificate/repositorie
 import { ClientCertificate } from 'src/modules/certificate/models/client-certificate';
 import { CreateClientCertificateDto } from 'src/modules/certificate/dto/create.client-certificate.dto';
 import { classToClass } from 'src/utils';
-import { SessionMetadata } from 'src/common/models';
-import { DatabaseRepository } from 'src/modules/database/repositories/database.repository';
 import { RedisClientStorage } from 'src/modules/redis/redis.client.storage';
 
 @Injectable()
@@ -22,7 +20,6 @@ export class ClientCertificateService {
 
   constructor(
     private readonly repository: ClientCertificateRepository,
-    private readonly databaseRepository: DatabaseRepository,
     private redisClientStorage: RedisClientStorage,
   ) {}
 
@@ -68,19 +65,16 @@ export class ClientCertificateService {
     }
   }
 
-  async delete(sessionMetadata: SessionMetadata, id: string): Promise<void> {
+  async delete(id: string): Promise<void> {
     this.logger.log(`Deleting client certificate. id: ${id}`);
 
     try {
-      const databases = await this.databaseRepository.list(sessionMetadata);
-      await Promise.all(databases.map(async (database) => {
-        const db = await this.databaseRepository.get(sessionMetadata, database.id);
-        if (db.clientCert?.id === id) {
-          // If the certificate is used by the database, remove the client
-          await this.redisClientStorage.removeManyByMetadata({ databaseId: db.id });
-        }
+      const { affectedDatabases } = await this.repository.delete(id);
+
+      await Promise.all(affectedDatabases.map(async (databaseId) => {
+        // If the certificate is used by the database, remove the client
+        await this.redisClientStorage.removeManyByMetadata({ databaseId });
       }));
-      await this.repository.delete(id);
     } catch (error) {
       this.logger.error(`Failed to delete certificate ${id}`, error);
 

redisinsight/api/src/modules/certificate/repositories/ca-certificate.repository.ts

@@ -26,5 +26,5 @@ export abstract class CaCertificateRepository {
    * @param id
    * @throws NotFoundException in case when try to delete not existing cert (?)
    */
-  abstract delete(id: string): Promise<void>;
+  abstract delete(id: string): Promise<{ affectedDatabases: string[] }>;
 }

redisinsight/api/src/modules/certificate/repositories/client-certificate.repository.ts

@@ -24,5 +24,5 @@ export abstract class ClientCertificateRepository {
    * @param id
    * @throws NotFoundException in case when try to delete not existing cert (?)
    */
-  abstract delete(id: string): Promise<void>;
+  abstract delete(id: string): Promise<{ affectedDatabases: string[] }>;
 }

redisinsight/api/src/modules/certificate/repositories/local.ca-certificate.repository.ts

@@ -13,6 +13,7 @@ import ERROR_MESSAGES from 'src/constants/error-messages';
 import { classToClass } from 'src/utils';
 import { EncryptionService } from 'src/modules/encryption/encryption.service';
 import { ModelEncryptor } from 'src/modules/encryption/model.encryptor';
+import { DatabaseEntity } from 'src/modules/database/entities/database.entity';
 
 @Injectable()
 export class LocalCaCertificateRepository extends CaCertificateRepository {
@@ -23,6 +24,8 @@ export class LocalCaCertificateRepository extends CaCertificateRepository {
   constructor(
     @InjectRepository(CaCertificateEntity)
     private readonly repository: Repository<CaCertificateEntity>,
+    @InjectRepository(DatabaseEntity)
+    private readonly databaseRepository: Repository<DatabaseEntity>,
     private readonly encryptionService: EncryptionService,
   ) {
     super();
@@ -71,7 +74,7 @@ export class LocalCaCertificateRepository extends CaCertificateRepository {
   /**
    * @inheritDoc
    */
-  async delete(id: string): Promise<void> {
+  async delete(id: string): Promise<{ affectedDatabases: string[] }> {
     this.logger.log(`Deleting certificate. id: ${id}`);
 
     // todo: 1. why we need to check if entity exists?
@@ -84,7 +87,16 @@ export class LocalCaCertificateRepository extends CaCertificateRepository {
       throw new NotFoundException();
     }
 
+    const affectedDatabases = (await this.databaseRepository
+      .createQueryBuilder('d')
+      .leftJoinAndSelect('d.caCert', 'c')
+      .where({ caCert: id })
+      .select(['d.id'])
+      .getMany()).map((e) => e.id);
+
     await this.repository.delete(id);
     this.logger.log(`Succeed to delete ca certificate: ${id}`);
+
+    return { affectedDatabases };
   }
 }

redisinsight/api/src/modules/certificate/repositories/local.client-certificate.repository.ts

@@ -13,6 +13,7 @@ import { ModelEncryptor } from 'src/modules/encryption/model.encryptor';
 import { ClientCertificateRepository } from 'src/modules/certificate/repositories/client-certificate.repository';
 import { ClientCertificateEntity } from 'src/modules/certificate/entities/client-certificate.entity';
 import { ClientCertificate } from 'src/modules/certificate/models/client-certificate';
+import { DatabaseEntity } from 'src/modules/database/entities/database.entity';
 
 @Injectable()
 export class LocalClientCertificateRepository extends ClientCertificateRepository {
@@ -23,6 +24,8 @@ export class LocalClientCertificateRepository extends ClientCertificateRepositor
   constructor(
     @InjectRepository(ClientCertificateEntity)
     private readonly repository: Repository<ClientCertificateEntity>,
+    @InjectRepository(DatabaseEntity)
+    private readonly databaseRepository: Repository<DatabaseEntity>,
     private readonly encryptionService: EncryptionService,
   ) {
     super();
@@ -71,7 +74,7 @@ export class LocalClientCertificateRepository extends ClientCertificateRepositor
   /**
    * @inheritDoc
    */
-  async delete(id: string): Promise<void> {
+  async delete(id: string): Promise<{ affectedDatabases: string[] }> {
     this.logger.log(`Deleting certificate. id: ${id}`);
 
     // todo: 1. why we need to check if entity exists?
@@ -83,7 +86,16 @@ export class LocalClientCertificateRepository extends ClientCertificateRepositor
       throw new NotFoundException();
     }
 
+    const affectedDatabases = (await this.databaseRepository
+      .createQueryBuilder('d')
+      .leftJoinAndSelect('d.clientCert', 'c')
+      .where({ clientCert: id })
+      .select(['d.id'])
+      .getMany()).map((e) => e.id);
+
     await this.repository.delete(id);
     this.logger.log(`Succeed to delete client certificate: ${id}`);
+
+    return { affectedDatabases };
   }
 }