refactor(core): Move more typeorm operators to repositories (no-cha…

…ngelog) (n8n-io#8143) Follow-up to n8n-io#8139
jamesfdavis · Dec 22, 2023 · a59d78d · a59d78d
1 parent 4007163
commit a59d78d
Show file tree

Hide file tree

Showing 11 changed files with 82 additions and 56 deletions.
diff --git a/packages/cli/src/credentials/credentials.controller.ee.ts b/packages/cli/src/credentials/credentials.controller.ee.ts
@@ -14,6 +14,7 @@ import type { CredentialsEntity } from '@db/entities/CredentialsEntity';
 import { BadRequestError } from '@/errors/response-errors/bad-request.error';
 import { NotFoundError } from '@/errors/response-errors/not-found.error';
 import { UnauthorizedError } from '@/errors/response-errors/unauthorized.error';
+import { CredentialsRepository } from '@/databases/repositories/credentials.repository';
 
 export const EECredentialsController = express.Router();
 
@@ -155,10 +156,11 @@ EECredentialsController.put(
 		let newShareeIds: string[] = [];
 		await Db.transaction(async (trx) => {
 			// remove all sharings that are not supposed to exist anymore
-			const { affected } = await EECredentials.pruneSharings(trx, credentialId, [
-				...ownerIds,
-				...shareWithIds,
-			]);
+			const { affected } = await Container.get(CredentialsRepository).pruneSharings(
+				trx,
+				credentialId,
+				[...ownerIds, ...shareWithIds],
+			);
 			if (affected) amountRemoved = affected;
 
 			const sharings = await EECredentials.getSharings(trx, credentialId);

diff --git a/packages/cli/src/credentials/credentials.service.ee.ts b/packages/cli/src/credentials/credentials.service.ee.ts
@@ -1,7 +1,6 @@
-import type { DeleteResult, EntityManager, FindOptionsWhere } from 'typeorm';
-import { In, Not } from 'typeorm';
+import type { EntityManager, FindOptionsWhere } from 'typeorm';
 import { CredentialsEntity } from '@db/entities/CredentialsEntity';
-import { SharedCredentials } from '@db/entities/SharedCredentials';
+import type { SharedCredentials } from '@db/entities/SharedCredentials';
 import type { User } from '@db/entities/User';
 import { UserService } from '@/services/user.service';
 import { CredentialsService, type CredentialsGetSharedOptions } from './credentials.service';
@@ -62,18 +61,6 @@ export class EECredentialsService extends CredentialsService {
 		return credential?.shared ?? [];
 	}
 
-	static async pruneSharings(
-		transaction: EntityManager,
-		credentialId: string,
-		userIds: string[],
-	): Promise<DeleteResult> {
-		const conditions: FindOptionsWhere<SharedCredentials> = {
-			credentialsId: credentialId,
-			userId: Not(In(userIds)),
-		};
-		return transaction.delete(SharedCredentials, conditions);
-	}
-
 	static async share(
 		transaction: EntityManager,
 		credential: CredentialsEntity,

diff --git a/packages/cli/src/databases/repositories/credentials.repository.ts b/packages/cli/src/databases/repositories/credentials.repository.ts
@@ -1,10 +1,39 @@
 import { Service } from 'typedi';
-import { DataSource, Repository } from 'typeorm';
+import {
+	DataSource,
+	In,
+	Not,
+	Repository,
+	type DeleteResult,
+	type EntityManager,
+	type FindOptionsWhere,
+	Like,
+} from 'typeorm';
 import { CredentialsEntity } from '../entities/CredentialsEntity';
+import { SharedCredentials } from '../entities/SharedCredentials';
 
 @Service()
 export class CredentialsRepository extends Repository<CredentialsEntity> {
 	constructor(dataSource: DataSource) {
 		super(CredentialsEntity, dataSource.manager);
 	}
+
+	async pruneSharings(
+		transaction: EntityManager,
+		credentialId: string,
+		userIds: string[],
+	): Promise<DeleteResult> {
+		const conditions: FindOptionsWhere<SharedCredentials> = {
+			credentialsId: credentialId,
+			userId: Not(In(userIds)),
+		};
+		return transaction.delete(SharedCredentials, conditions);
+	}
+
+	async findStartingWith(credentialName: string) {
+		return this.find({
+			select: ['name'],
+			where: { name: Like(`${credentialName}%`) },
+		});
+	}
 }
diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts
@@ -425,4 +425,13 @@ export class ExecutionRepository extends Repository<ExecutionEntity> {
 			await this.delete(batch);
 		} while (executionIds.length > 0);
 	}
+
+	async getIdsSince(date: Date) {
+		return this.find({
+			select: ['id'],
+			where: {
+				startedAt: MoreThanOrEqual(DateUtils.mixedDateToUtcDatetimeString(date)),
+			},
+		}).then((executions) => executions.map(({ id }) => id));
+	}
 }
diff --git a/packages/cli/src/databases/repositories/executionData.repository.ts b/packages/cli/src/databases/repositories/executionData.repository.ts
@@ -1,10 +1,19 @@
 import { Service } from 'typedi';
-import { DataSource, Repository } from 'typeorm';
+import { DataSource, In, Repository } from 'typeorm';
 import { ExecutionData } from '../entities/ExecutionData';
 
 @Service()
 export class ExecutionDataRepository extends Repository<ExecutionData> {
 	constructor(dataSource: DataSource) {
 		super(ExecutionData, dataSource.manager);
 	}
+
+	async findByExecutionIds(executionIds: string[]) {
+		return this.find({
+			select: ['workflowData'],
+			where: {
+				executionId: In(executionIds),
+			},
+		}).then((executionData) => executionData.map(({ workflowData }) => workflowData));
+	}
 }
diff --git a/packages/cli/src/databases/repositories/workflow.repository.ts b/packages/cli/src/databases/repositories/workflow.repository.ts
@@ -172,4 +172,11 @@ export class WorkflowRepository extends Repository<WorkflowEntity> {
 
 		return { workflows, count };
 	}
+
+	async findStartingWith(workflowName: string): Promise<Array<{ name: string }>> {
+		return this.find({
+			select: ['name'],
+			where: { name: Like(`${workflowName}%`) },
+		});
+	}
 }
diff --git a/packages/cli/src/databases/repositories/workflowHistory.repository.ts b/packages/cli/src/databases/repositories/workflowHistory.repository.ts
@@ -1,10 +1,14 @@
 import { Service } from 'typedi';
-import { DataSource, Repository } from 'typeorm';
+import { DataSource, LessThan, Repository } from 'typeorm';
 import { WorkflowHistory } from '../entities/WorkflowHistory';
 
 @Service()
 export class WorkflowHistoryRepository extends Repository<WorkflowHistory> {
 	constructor(dataSource: DataSource) {
 		super(WorkflowHistory, dataSource.manager);
 	}
+
+	async deleteEarlierThan(date: Date) {
+		return this.delete({ createdAt: LessThan(date) });
+	}
 }
diff --git a/packages/cli/src/security-audit/risk-reporters/CredentialsRiskReporter.ts b/packages/cli/src/security-audit/risk-reporters/CredentialsRiskReporter.ts
@@ -1,5 +1,3 @@
-import { In, MoreThanOrEqual } from 'typeorm';
-import { DateUtils } from 'typeorm/util/DateUtils';
 import { Service } from 'typedi';
 import type { IWorkflowBase } from 'n8n-workflow';
 import config from '@/config';
@@ -119,23 +117,9 @@ export class CredentialsRiskReporter implements RiskReporter {
 
 		date.setDate(date.getDate() - days);
 
-		const executionIds = await this.executionRepository
-			.find({
-				select: ['id'],
-				where: {
-					startedAt: MoreThanOrEqual(DateUtils.mixedDateToUtcDatetimeString(date) as Date),
-				},
-			})
-			.then((executions) => executions.map(({ id }) => id));
-
-		return this.executionDataRepository
-			.find({
-				select: ['workflowData'],
-				where: {
-					executionId: In(executionIds),
-				},
-			})
-			.then((executionData) => executionData.map(({ workflowData }) => workflowData));
+		const executionIds = await this.executionRepository.getIdsSince(date);
+
+		return this.executionDataRepository.findByExecutionIds(executionIds);
 	}
 
 	/**

diff --git a/packages/cli/src/services/naming.service.ts b/packages/cli/src/services/naming.service.ts
@@ -1,5 +1,4 @@
 import { Service } from 'typedi';
-import { Like } from 'typeorm';
 import { WorkflowRepository } from '@/databases/repositories/workflow.repository';
 import { CredentialsRepository } from '@/databases/repositories/credentials.repository';
 
@@ -21,10 +20,7 @@ export class NamingService {
 	private async getUniqueName(requestedName: string, entity: 'workflow' | 'credential') {
 		const repository = entity === 'workflow' ? this.workflowRepository : this.credentialsRepository;
 
-		const found: Array<{ name: string }> = await repository.find({
-			select: ['name'],
-			where: { name: Like(`${requestedName}%`) },
-		});
+		const found = await repository.findStartingWith(requestedName);
 
 		if (found.length === 0) return requestedName;
 

diff --git a/packages/cli/src/workflows/workflowHistory/workflowHistoryManager.ee.ts b/packages/cli/src/workflows/workflowHistory/workflowHistoryManager.ee.ts
@@ -1,5 +1,4 @@
 import { Service } from 'typedi';
-import { LessThan } from 'typeorm';
 import { DateTime } from 'luxon';
 import { WorkflowHistoryRepository } from '@db/repositories/workflowHistory.repository';
 import { WORKFLOW_HISTORY_PRUNE_INTERVAL } from './constants';
@@ -38,8 +37,6 @@ export class WorkflowHistoryManager {
 		}
 		const pruneDateTime = DateTime.now().minus({ hours: pruneHours }).toJSDate();
 
-		await this.workflowHistoryRepo.delete({
-			createdAt: LessThan(pruneDateTime),
-		});
+		await this.workflowHistoryRepo.deleteEarlierThan(pruneDateTime);
 	}
 }
diff --git a/packages/cli/test/unit/services/naming.service.test.ts b/packages/cli/test/unit/services/naming.service.test.ts
@@ -17,15 +17,15 @@ describe('NamingService', () => {
 
 	describe('getUniqueWorkflowName()', () => {
 		test('should return requested name if already unique', async () => {
-			workflowRepository.find.mockResolvedValue([]);
+			workflowRepository.findStartingWith.mockResolvedValue([]);
 
 			const name = await namingService.getUniqueWorkflowName('foo');
 
 			expect(name).toEqual('foo');
 		});
 
 		test('should return requested name suffixed if already existing once', async () => {
-			workflowRepository.find.mockResolvedValue([{ name: 'foo' }] as WorkflowEntity[]);
+			workflowRepository.findStartingWith.mockResolvedValue([{ name: 'foo' }] as WorkflowEntity[]);
 
 			const name = await namingService.getUniqueWorkflowName('foo');
 
@@ -35,7 +35,7 @@ describe('NamingService', () => {
 		test('should return requested name with incremented suffix if already suffixed', async () => {
 			const existingNames = [{ name: 'foo' }, { name: 'foo 2' }] as WorkflowEntity[];
 
-			workflowRepository.find.mockResolvedValue(existingNames);
+			workflowRepository.findStartingWith.mockResolvedValue(existingNames);
 
 			const name = await namingService.getUniqueWorkflowName('foo');
 
@@ -51,15 +51,17 @@ describe('NamingService', () => {
 
 	describe('getUniqueCredentialName()', () => {
 		test('should return requested name if already unique', async () => {
-			credentialsRepository.find.mockResolvedValue([]);
+			credentialsRepository.findStartingWith.mockResolvedValue([]);
 
 			const name = await namingService.getUniqueCredentialName('foo');
 
 			expect(name).toEqual('foo');
 		});
 
 		test('should return requested name suffixed if already existing once', async () => {
-			credentialsRepository.find.mockResolvedValue([{ name: 'foo' }] as CredentialsEntity[]);
+			credentialsRepository.findStartingWith.mockResolvedValue([
+				{ name: 'foo' },
+			] as CredentialsEntity[]);
 
 			const name = await namingService.getUniqueCredentialName('foo');
 
@@ -69,7 +71,7 @@ describe('NamingService', () => {
 		test('should return requested name with incremented suffix if already suffixed', async () => {
 			const existingNames = [{ name: 'foo' }, { name: 'foo 2' }] as CredentialsEntity[];
 
-			credentialsRepository.find.mockResolvedValue(existingNames);
+			credentialsRepository.findStartingWith.mockResolvedValue(existingNames);
 
 			const name = await namingService.getUniqueCredentialName('foo');