From 31c5486aad3c264d708f3dc2268a1fea958f9fba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9ctor=20G=C3=B3mez?= Date: Tue, 28 Nov 2023 14:43:15 +0100 Subject: [PATCH] Add a configurable webhook execution delay (#883) Changes: - Adds a configurable (via the environment variable `WEB_HOOK_EXECUTION_DELAY_MS`) delay before executing web hook. The default value for this is 5 seconds. - The status code returned by the controller was changed to 202 as it is more accurate given the new behavior. --- .../entities/__tests__/configuration.ts | 1 + src/config/entities/configuration.ts | 3 + .../cache-hooks.controller.spec.ts | 102 +++++++++++++----- .../cache-hooks/cache-hooks.controller.ts | 29 ++++- 4 files changed, 104 insertions(+), 31 deletions(-) diff --git a/src/config/entities/__tests__/configuration.ts b/src/config/entities/__tests__/configuration.ts index 7e1e9c2ee1..49c4dd8162 100644 --- a/src/config/entities/__tests__/configuration.ts +++ b/src/config/entities/__tests__/configuration.ts @@ -141,4 +141,5 @@ export default (): ReturnType => ({ safeTransaction: { useVpcUrl: false, }, + webHookExecutionDelayMs: faker.number.int({ min: 1, max: 10 }), }); diff --git a/src/config/entities/configuration.ts b/src/config/entities/configuration.ts index d2a2f5ceb9..3a886d65cb 100644 --- a/src/config/entities/configuration.ts +++ b/src/config/entities/configuration.ts @@ -130,4 +130,7 @@ export default () => ({ safeTransaction: { useVpcUrl: process.env.USE_TX_SERVICE_VPC_URL?.toLowerCase() === 'true', }, + webHookExecutionDelayMs: parseInt( + process.env.WEB_HOOK_EXECUTION_DELAY_MS ?? `${5_000}`, + ), }); diff --git a/src/routes/cache-hooks/cache-hooks.controller.spec.ts b/src/routes/cache-hooks/cache-hooks.controller.spec.ts index c7f0d9a702..eb58141389 100644 --- a/src/routes/cache-hooks/cache-hooks.controller.spec.ts +++ b/src/routes/cache-hooks/cache-hooks.controller.spec.ts @@ -25,9 +25,11 @@ describe('Post Hook Events (Unit)', () => { let safeConfigUrl; let fakeCacheService: FakeCacheService; let networkService; + let webHookExecutionDelayMs; beforeEach(async () => { jest.clearAllMocks(); + jest.useFakeTimers(); const moduleFixture: TestingModule = await Test.createTestingModule({ imports: [AppModule.register(configuration)], @@ -47,11 +49,18 @@ describe('Post Hook Events (Unit)', () => { const configurationService = moduleFixture.get(IConfigurationService); authToken = configurationService.get('auth.token'); safeConfigUrl = configurationService.get('safeConfig.baseUri'); + webHookExecutionDelayMs = configurationService.get( + 'webHookExecutionDelayMs', + ); networkService = moduleFixture.get(NetworkService); await app.init(); }); + afterEach(() => { + jest.useRealTimers(); + }); + afterAll(async () => { await app.close(); }); @@ -148,7 +157,7 @@ describe('Post Hook Events (Unit)', () => { .post(`/hooks/events`) .set('Authorization', `Basic ${authToken}`) .send(data) - .expect(200); + .expect(202); }); it('returns 400 (Bad Request) on unknown payload', async () => { @@ -223,9 +232,12 @@ describe('Post Hook Events (Unit)', () => { .post(`/hooks/events`) .set('Authorization', `Basic ${authToken}`) .send(data) - .expect(200); + .expect(202); - await expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(); + setTimeout( + () => expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(), + webHookExecutionDelayMs, + ); }); it.each([ @@ -271,9 +283,12 @@ describe('Post Hook Events (Unit)', () => { .post(`/hooks/events`) .set('Authorization', `Basic ${authToken}`) .send(data) - .expect(200); + .expect(202); - await expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(); + setTimeout( + () => expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(), + webHookExecutionDelayMs, + ); }); it.each([ @@ -319,9 +334,12 @@ describe('Post Hook Events (Unit)', () => { .post(`/hooks/events`) .set('Authorization', `Basic ${authToken}`) .send(data) - .expect(200); + .expect(202); - await expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(); + setTimeout( + () => expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(), + webHookExecutionDelayMs, + ); }); it.each([ @@ -358,9 +376,12 @@ describe('Post Hook Events (Unit)', () => { .post(`/hooks/events`) .set('Authorization', `Basic ${authToken}`) .send(data) - .expect(200); + .expect(202); - await expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(); + setTimeout( + () => expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(), + webHookExecutionDelayMs, + ); }); it.each([ @@ -407,9 +428,12 @@ describe('Post Hook Events (Unit)', () => { .post(`/hooks/events`) .set('Authorization', `Basic ${authToken}`) .send(data) - .expect(200); + .expect(202); - await expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(); + setTimeout( + () => expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(), + webHookExecutionDelayMs, + ); }); it.each([ @@ -456,9 +480,12 @@ describe('Post Hook Events (Unit)', () => { .post(`/hooks/events`) .set('Authorization', `Basic ${authToken}`) .send(data) - .expect(200); + .expect(202); - await expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(); + setTimeout( + () => expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(), + webHookExecutionDelayMs, + ); }); it.each([ @@ -500,9 +527,12 @@ describe('Post Hook Events (Unit)', () => { .post(`/hooks/events`) .set('Authorization', `Basic ${authToken}`) .send(data) - .expect(200); + .expect(202); - await expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(); + setTimeout( + () => expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(), + webHookExecutionDelayMs, + ); }); it.each([ @@ -539,9 +569,12 @@ describe('Post Hook Events (Unit)', () => { .post(`/hooks/events`) .set('Authorization', `Basic ${authToken}`) .send(data) - .expect(200); + .expect(202); - await expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(); + setTimeout( + () => expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(), + webHookExecutionDelayMs, + ); }); it.each([ @@ -603,9 +636,12 @@ describe('Post Hook Events (Unit)', () => { .post(`/hooks/events`) .set('Authorization', `Basic ${authToken}`) .send(data) - .expect(200); + .expect(202); - await expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(); + setTimeout( + () => expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(), + webHookExecutionDelayMs, + ); }); it.each([ @@ -645,9 +681,12 @@ describe('Post Hook Events (Unit)', () => { .post(`/hooks/events`) .set('Authorization', `Basic ${authToken}`) .send(data) - .expect(200); + .expect(202); - await expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(); + setTimeout( + () => expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(), + webHookExecutionDelayMs, + ); }); it.each([ @@ -667,9 +706,12 @@ describe('Post Hook Events (Unit)', () => { .post(`/hooks/events`) .set('Authorization', `Basic ${authToken}`) .send(data) - .expect(200); + .expect(202); - await expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(); + setTimeout( + () => expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(), + webHookExecutionDelayMs, + ); }); it.each([ @@ -689,9 +731,12 @@ describe('Post Hook Events (Unit)', () => { .post(`/hooks/events`) .set('Authorization', `Basic ${authToken}`) .send(data) - .expect(200); + .expect(202); - await expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(); + setTimeout( + () => expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(), + webHookExecutionDelayMs, + ); }); it.each([ @@ -711,8 +756,11 @@ describe('Post Hook Events (Unit)', () => { .post(`/hooks/events`) .set('Authorization', `Basic ${authToken}`) .send(data) - .expect(200); + .expect(202); - await expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(); + setTimeout( + () => expect(fakeCacheService.get(cacheDir)).resolves.toBeNull(), + webHookExecutionDelayMs, + ); }); }); diff --git a/src/routes/cache-hooks/cache-hooks.controller.ts b/src/routes/cache-hooks/cache-hooks.controller.ts index c4afc2972f..71547024dd 100644 --- a/src/routes/cache-hooks/cache-hooks.controller.ts +++ b/src/routes/cache-hooks/cache-hooks.controller.ts @@ -1,4 +1,11 @@ -import { Body, Controller, HttpCode, Post, UseGuards } from '@nestjs/common'; +import { + Body, + Controller, + HttpCode, + Inject, + Post, + UseGuards, +} from '@nestjs/common'; import { ApiExcludeController } from '@nestjs/swagger'; import { CacheHooksService } from '@/routes/cache-hooks/cache-hooks.service'; import { ChainUpdate } from '@/routes/cache-hooks/entities/chain-update.entity'; @@ -15,6 +22,7 @@ import { PendingTransaction } from '@/routes/cache-hooks/entities/pending-transa import { SafeAppsUpdate } from '@/routes/cache-hooks/entities/safe-apps-update.entity'; import { EventValidationPipe } from '@/routes/cache-hooks/pipes/event-validation.pipe'; import { BasicAuthGuard } from '@/routes/common/auth/basic-auth.guard'; +import { IConfigurationService } from '@/config/configuration.service.interface'; @Controller({ path: '', @@ -22,11 +30,21 @@ import { BasicAuthGuard } from '@/routes/common/auth/basic-auth.guard'; }) @ApiExcludeController() export class CacheHooksController { - constructor(private readonly service: CacheHooksService) {} + private readonly webHookExecutionDelayMs: number; + + constructor( + private readonly service: CacheHooksService, + @Inject(IConfigurationService) + private readonly configurationService: IConfigurationService, + ) { + this.webHookExecutionDelayMs = this.configurationService.getOrThrow( + 'webHookExecutionDelayMs', + ); + } @UseGuards(BasicAuthGuard) @Post('/hooks/events') - @HttpCode(200) + @HttpCode(202) async postEvent( @Body(EventValidationPipe) eventPayload: @@ -43,6 +61,9 @@ export class CacheHooksController { | PendingTransaction | SafeAppsUpdate, ): Promise { - await this.service.onEvent(eventPayload); + setTimeout( + () => this.service.onEvent(eventPayload), + this.webHookExecutionDelayMs, + ); } }