From 9e5f66be5d69cc0e15e63d9e99ca96b2b049e48c Mon Sep 17 00:00:00 2001 From: Danny Martini Date: Mon, 9 Dec 2024 16:22:14 +0100 Subject: [PATCH 1/2] feat(core): Add option to filter for empty variables --- packages/@n8n/api-types/src/dto/index.ts | 1 + .../variables/variables-list-request.dto.ts | 6 ++++++ .../variables/variables.controller.ee.ts | 7 +++++-- .../variables/variables.service.ee.ts | 15 ++++++++++++--- .../cli/test/integration/variables.test.ts | 19 ++++++++++++++++--- 5 files changed, 40 insertions(+), 8 deletions(-) create mode 100644 packages/@n8n/api-types/src/dto/variables/variables-list-request.dto.ts diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts index 41a55f050a358..97d5d38459b4d 100644 --- a/packages/@n8n/api-types/src/dto/index.ts +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -3,3 +3,4 @@ export { RoleChangeRequestDto } from './user/role-change-request.dto'; export { SettingsUpdateRequestDto } from './user/settings-update-request.dto'; export { UserUpdateRequestDto } from './user/user-update-request.dto'; export { CommunityRegisteredRequestDto } from './license/community-registered-request.dto'; +export { VariableListRequestDto } from './variables/variables-list-request.dto'; diff --git a/packages/@n8n/api-types/src/dto/variables/variables-list-request.dto.ts b/packages/@n8n/api-types/src/dto/variables/variables-list-request.dto.ts new file mode 100644 index 0000000000000..804bcb27865e9 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/variables/variables-list-request.dto.ts @@ -0,0 +1,6 @@ +import { z } from 'zod'; +import { Z } from 'zod-class'; + +export class VariableListRequestDto extends Z.class({ + state: z.literal('empty').optional(), +}) {} diff --git a/packages/cli/src/environments/variables/variables.controller.ee.ts b/packages/cli/src/environments/variables/variables.controller.ee.ts index 5da4221a3d56d..a38906b800c26 100644 --- a/packages/cli/src/environments/variables/variables.controller.ee.ts +++ b/packages/cli/src/environments/variables/variables.controller.ee.ts @@ -1,4 +1,7 @@ +import { VariableListRequestDto } from '@n8n/api-types'; + import { Delete, Get, GlobalScope, Licensed, Patch, Post, RestController } from '@/decorators'; +import { Query } from '@/decorators/args'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { VariableCountLimitReachedError } from '@/errors/variable-count-limit-reached.error'; @@ -13,8 +16,8 @@ export class VariablesController { @Get('/') @GlobalScope('variable:list') - async getVariables() { - return await this.variablesService.getAllCached(); + async getVariables(_req: unknown, _res: unknown, @Query query: VariableListRequestDto) { + return await this.variablesService.getAllCached(query.state); } @Post('/') diff --git a/packages/cli/src/environments/variables/variables.service.ee.ts b/packages/cli/src/environments/variables/variables.service.ee.ts index 31cf725099b90..38ad5703ea91e 100644 --- a/packages/cli/src/environments/variables/variables.service.ee.ts +++ b/packages/cli/src/environments/variables/variables.service.ee.ts @@ -18,13 +18,22 @@ export class VariablesService { private readonly eventService: EventService, ) {} - async getAllCached(): Promise { - const variables = await this.cacheService.get('variables', { + async getAllCached(state?: 'empty'): Promise { + let variables = await this.cacheService.get('variables', { async refreshFn() { return await Container.get(VariablesService).findAll(); }, }); - return (variables as Array>).map((v) => this.variablesRepository.create(v)); + + if (variables === undefined) { + return []; + } + + if (state === 'empty') { + variables = variables.filter((v) => v.value === ''); + } + + return variables.map((v) => this.variablesRepository.create(v)); } async getCount(): Promise { diff --git a/packages/cli/test/integration/variables.test.ts b/packages/cli/test/integration/variables.test.ts index f8cef375391a4..b29145410bc29 100644 --- a/packages/cli/test/integration/variables.test.ts +++ b/packages/cli/test/integration/variables.test.ts @@ -65,19 +65,32 @@ beforeEach(async () => { // ---------------------------------------- describe('GET /variables', () => { beforeEach(async () => { - await Promise.all([createVariable('test1', 'value1'), createVariable('test2', 'value2')]); + await Promise.all([ + createVariable('test1', 'value1'), + createVariable('test2', 'value2'), + createVariable('empty', ''), + ]); }); test('should return all variables for an owner', async () => { const response = await authOwnerAgent.get('/variables'); expect(response.statusCode).toBe(200); - expect(response.body.data.length).toBe(2); + expect(response.body.data.length).toBe(3); }); test('should return all variables for a member', async () => { const response = await authMemberAgent.get('/variables'); expect(response.statusCode).toBe(200); - expect(response.body.data.length).toBe(2); + expect(response.body.data.length).toBe(3); + }); + + describe('state:empty', () => { + test('only return empty variables', async () => { + const response = await authOwnerAgent.get('/variables').query({ state: 'empty' }); + expect(response.statusCode).toBe(200); + expect(response.body.data.length).toBe(1); + expect(response.body.data[0]).toMatchObject({ key: 'empty', value: '', type: 'string' }); + }); }); }); From 5ebd350cc9bed327b1e246949c6711c7b629e8be Mon Sep 17 00:00:00 2001 From: Danny Martini Date: Tue, 10 Dec 2024 10:33:48 +0100 Subject: [PATCH 2/2] add test to improve coverage --- packages/cli/test/integration/variables.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/cli/test/integration/variables.test.ts b/packages/cli/test/integration/variables.test.ts index b29145410bc29..7dd8d00aaedd8 100644 --- a/packages/cli/test/integration/variables.test.ts +++ b/packages/cli/test/integration/variables.test.ts @@ -4,6 +4,7 @@ import type { Variables } from '@/databases/entities/variables'; import { VariablesRepository } from '@/databases/repositories/variables.repository'; import { generateNanoId } from '@/databases/utils/generators'; import { VariablesService } from '@/environments/variables/variables.service.ee'; +import { CacheService } from '@/services/cache/cache.service'; import { createOwner, createUser } from './shared/db/users'; import * as testDb from './shared/test-db'; @@ -72,6 +73,15 @@ describe('GET /variables', () => { ]); }); + test('should return an empty array if there is nothing in the cache', async () => { + const cacheService = Container.get(CacheService); + const spy = jest.spyOn(cacheService, 'get').mockResolvedValueOnce(undefined); + const response = await authOwnerAgent.get('/variables'); + expect(spy).toHaveBeenCalledTimes(1); + expect(response.statusCode).toBe(200); + expect(response.body.data.length).toBe(0); + }); + test('should return all variables for an owner', async () => { const response = await authOwnerAgent.get('/variables'); expect(response.statusCode).toBe(200);