Skip to content

Commit 61a5261

Browse files
cstuncsikcstuncsik
authored
fix(core): Clean up resolver references on deletion (n8n-io#26524)
1 parent 28f50f5 commit 61a5261

File tree

18 files changed

+813
-16
lines changed

18 files changed

+813
-16
lines changed

‎packages/@n8n/api-types/src/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,10 @@ export {
113113
credentialResolversSchema,
114114
credentialResolverTypeSchema,
115115
credentialResolverTypesSchema,
116+
credentialResolverAffectedWorkflowsSchema,
116117
type CredentialResolver,
117118
type CredentialResolverType,
119+
type CredentialResolverAffectedWorkflow,
118120
} from './schemas/credential-resolver.schema';
119121
export {
120122
WORKFLOW_VERSION_NAME_MAX_LENGTH,

‎packages/@n8n/api-types/src/schemas/credential-resolver.schema.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,16 @@ export type CredentialResolverType = z.infer<typeof credentialResolverTypeSchema
2929
export const credentialResolversSchema = z.array(credentialResolverSchema);
3030

3131
export type CredentialResolver = z.infer<typeof credentialResolverSchema>;
32+
33+
export const credentialResolverAffectedWorkflowSchema = z.object({
34+
id: z.string(),
35+
name: z.string(),
36+
});
37+
38+
export const credentialResolverAffectedWorkflowsSchema = z.array(
39+
credentialResolverAffectedWorkflowSchema,
40+
);
41+
42+
export type CredentialResolverAffectedWorkflow = z.infer<
43+
typeof credentialResolverAffectedWorkflowSchema
44+
>;

‎packages/@n8n/db/src/repositories/__tests__/workflow.repository.test.ts‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,4 +567,105 @@ describe('WorkflowRepository', () => {
567567
expect(result).toBe(3);
568568
});
569569
});
570+
571+
describe('findByCredentialResolverId', () => {
572+
it('should use PostgreSQL JSON operator for postgresdb', async () => {
573+
const workflows = [{ id: 'wf-1', name: 'Workflow 1' }] as WorkflowEntity[];
574+
queryBuilder.getMany.mockResolvedValue(workflows);
575+
576+
const result = await workflowRepository.findByCredentialResolverId('resolver-123');
577+
578+
expect(queryBuilder.select).toHaveBeenCalledWith(['workflow.id', 'workflow.name']);
579+
expect(queryBuilder.where).toHaveBeenCalledWith(
580+
"workflow.settings ->> 'credentialResolverId' = :resolverId",
581+
{ resolverId: 'resolver-123' },
582+
);
583+
expect(result).toEqual(workflows);
584+
});
585+
586+
it('should use SQLite JSON_EXTRACT for sqlite', async () => {
587+
const sqliteConfig = mockInstance(GlobalConfig, {
588+
database: { type: 'sqlite' },
589+
});
590+
const sqliteWorkflowRepository = new WorkflowRepository(
591+
entityManager.connection,
592+
sqliteConfig,
593+
folderRepository,
594+
workflowHistoryRepository,
595+
);
596+
jest.spyOn(sqliteWorkflowRepository, 'createQueryBuilder').mockReturnValue(queryBuilder);
597+
queryBuilder.getMany.mockResolvedValue([]);
598+
599+
const result = await sqliteWorkflowRepository.findByCredentialResolverId('resolver-123');
600+
601+
expect(queryBuilder.where).toHaveBeenCalledWith(
602+
"JSON_EXTRACT(workflow.settings, '$.credentialResolverId') = :resolverId",
603+
{ resolverId: 'resolver-123' },
604+
);
605+
expect(result).toEqual([]);
606+
});
607+
608+
it('should return empty array when no workflows match', async () => {
609+
queryBuilder.getMany.mockResolvedValue([]);
610+
611+
const result = await workflowRepository.findByCredentialResolverId('no-match');
612+
613+
expect(result).toEqual([]);
614+
});
615+
});
616+
617+
describe('clearCredentialResolverId', () => {
618+
it('should use PostgreSQL jsonb removal for postgresdb', async () => {
619+
const mockExecute = jest.fn().mockResolvedValue({ affected: 1 });
620+
const mockUpdateWhere = jest.fn().mockReturnValue({ execute: mockExecute });
621+
const mockSet = jest.fn().mockReturnValue({ where: mockUpdateWhere });
622+
const mockUpdate = jest.fn().mockReturnValue({ set: mockSet });
623+
const updateQb = { update: mockUpdate } as unknown as SelectQueryBuilder<WorkflowEntity>;
624+
625+
jest.spyOn(workflowRepository, 'createQueryBuilder').mockReturnValue(updateQb);
626+
627+
await workflowRepository.clearCredentialResolverId('resolver-123');
628+
629+
expect(mockUpdate).toHaveBeenCalled();
630+
expect(mockSet).toHaveBeenCalledWith({
631+
settings: expect.any(Function),
632+
});
633+
expect(mockUpdateWhere).toHaveBeenCalledWith(
634+
"settings ->> 'credentialResolverId' = :resolverId",
635+
{ resolverId: 'resolver-123' },
636+
);
637+
expect(mockExecute).toHaveBeenCalled();
638+
});
639+
640+
it('should use SQLite json_remove for sqlite', async () => {
641+
const mockExecute = jest.fn().mockResolvedValue({ affected: 1 });
642+
const mockUpdateWhere = jest.fn().mockReturnValue({ execute: mockExecute });
643+
const mockSet = jest.fn().mockReturnValue({ where: mockUpdateWhere });
644+
const mockUpdate = jest.fn().mockReturnValue({ set: mockSet });
645+
const updateQb = { update: mockUpdate } as unknown as SelectQueryBuilder<WorkflowEntity>;
646+
647+
const sqliteConfig = mockInstance(GlobalConfig, {
648+
database: { type: 'sqlite' },
649+
});
650+
const sqliteWorkflowRepository = new WorkflowRepository(
651+
entityManager.connection,
652+
sqliteConfig,
653+
folderRepository,
654+
workflowHistoryRepository,
655+
);
656+
jest.spyOn(sqliteWorkflowRepository, 'createQueryBuilder').mockReturnValue(updateQb);
657+
658+
await sqliteWorkflowRepository.clearCredentialResolverId('resolver-123');
659+
660+
expect(mockUpdate).toHaveBeenCalled();
661+
expect(mockSet).toHaveBeenCalledWith({
662+
settings: expect.any(Function),
663+
});
664+
expect(mockUpdateWhere).toHaveBeenCalledWith(
665+
"JSON_EXTRACT(settings, '$.credentialResolverId') = :resolverId",
666+
{ resolverId: 'resolver-123' },
667+
);
668+
expect(mockExecute).toHaveBeenCalled();
669+
});
670+
});
570671
});

‎packages/@n8n/db/src/repositories/workflow.repository.ts‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,67 @@ export class WorkflowRepository extends Repository<WorkflowEntity> {
125125
return count > 0;
126126
}
127127

128+
async findByCredentialResolverId(
129+
resolverId: string,
130+
): Promise<Array<Pick<WorkflowEntity, 'id' | 'name'>>> {
131+
const qb = this.createQueryBuilder('workflow').select(['workflow.id', 'workflow.name']);
132+
this.addCredentialResolverFilter(qb, resolverId);
133+
return await qb.getMany();
134+
}
135+
136+
/**
137+
* Finds IDs of active workflows that reference a credential resolver.
138+
*/
139+
async findActiveByCredentialResolverId(resolverId: string): Promise<string[]> {
140+
const qb = this.createQueryBuilder('workflow')
141+
.select(['workflow.id'])
142+
.where('workflow.activeVersionId IS NOT NULL');
143+
this.addCredentialResolverFilter(qb, resolverId, 'andWhere');
144+
const workflows = await qb.getMany();
145+
return workflows.map((w) => w.id);
146+
}
147+
148+
private addCredentialResolverFilter(
149+
qb: SelectQueryBuilder<WorkflowEntity>,
150+
resolverId: string,
151+
method: 'where' | 'andWhere' = 'where',
152+
): void {
153+
const dbType = this.globalConfig.database.type;
154+
155+
if (dbType === 'postgresdb') {
156+
qb[method]("workflow.settings ->> 'credentialResolverId' = :resolverId", { resolverId });
157+
} else if (dbType === 'sqlite') {
158+
qb[method]("JSON_EXTRACT(workflow.settings, '$.credentialResolverId') = :resolverId", {
159+
resolverId,
160+
});
161+
}
162+
}
163+
164+
async clearCredentialResolverId(resolverId: string, trx?: EntityManager): Promise<void> {
165+
const dbType = this.globalConfig.database.type;
166+
const qb = trx
167+
? trx.createQueryBuilder().update(WorkflowEntity)
168+
: this.createQueryBuilder('workflow').update();
169+
170+
if (dbType === 'postgresdb') {
171+
await qb
172+
.set({
173+
settings: () => "settings::jsonb - 'credentialResolverId'",
174+
})
175+
.where("settings ->> 'credentialResolverId' = :resolverId", { resolverId })
176+
.execute();
177+
} else if (dbType === 'sqlite') {
178+
await qb
179+
.set({
180+
settings: () => "json_remove(settings, '$.credentialResolverId')",
181+
})
182+
.where("JSON_EXTRACT(settings, '$.credentialResolverId') = :resolverId", {
183+
resolverId,
184+
})
185+
.execute();
186+
}
187+
}
188+
128189
async findById(workflowId: string) {
129190
return await this.findOne({
130191
where: { id: workflowId },

‎packages/cli/src/modules/dynamic-credentials.ee/credential-resolvers.controller.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@ import {
33
CredentialResolver,
44
credentialResolverSchema,
55
credentialResolversSchema,
6+
credentialResolverAffectedWorkflowsSchema,
67
UpdateCredentialResolverDto,
78
CredentialResolverType,
89
credentialResolverTypesSchema,
10+
type CredentialResolverAffectedWorkflow,
911
} from '@n8n/api-types';
1012
import { AuthenticatedRequest } from '@n8n/db';
1113
import {
@@ -90,6 +92,27 @@ export class CredentialResolversController {
9092
}
9193
}
9294

95+
@Get('/:id/workflows')
96+
@GlobalScope('credentialResolver:read')
97+
async getAffectedWorkflows(
98+
_req: AuthenticatedRequest,
99+
_res: Response,
100+
@Param('id') id: string,
101+
): Promise<CredentialResolverAffectedWorkflow[]> {
102+
try {
103+
const workflows = await this.service.findAffectedWorkflows(id);
104+
return credentialResolverAffectedWorkflowsSchema.parse(workflows);
105+
} catch (e: unknown) {
106+
if (e instanceof DynamicCredentialResolverNotFoundError) {
107+
throw new NotFoundError(e.message);
108+
}
109+
if (e instanceof Error) {
110+
throw new InternalServerError(e.message, e);
111+
}
112+
throw e;
113+
}
114+
}
115+
93116
@Get('/:id')
94117
@GlobalScope('credentialResolver:read')
95118
async getResolver(

0 commit comments

Comments
 (0)