From ee87e05e2f5eb61b00b423d6394be9a131f84f8a Mon Sep 17 00:00:00 2001 From: Jay McDoniel Date: Mon, 1 Jun 2020 08:28:08 -0700 Subject: [PATCH] fix: updates tests and resolves comments from pr --- package.json | 2 +- src/throttle.decorator.ts | 6 +++--- src/throttler.guard.ts | 32 +++++++------------------------- test/app.e2e-spec.ts | 30 +++++++++--------------------- test/app/app.module.ts | 2 +- test/app/main.ts | 2 +- 6 files changed, 22 insertions(+), 52 deletions(-) diff --git a/package.json b/package.json index 2f2160b2..68c2342d 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ "test:cov": "jest --coverage", "test:debug": "node --inspect-brk -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --runInBand", "test:e2e": "jest --config ./test/jest-e2e.json", - "test:e2e:dev": "yarn test:e2e --watchAll --debug" + "test:e2e:dev": "yarn test:e2e --watchAll" }, "dependencies": { "@golevelup/nestjs-modules": "^0.4.1", diff --git a/src/throttle.decorator.ts b/src/throttle.decorator.ts index d143f8bf..c1b436ad 100644 --- a/src/throttle.decorator.ts +++ b/src/throttle.decorator.ts @@ -20,17 +20,17 @@ export const Throttle = (limit = 20, ttl = 60): MethodDecorator & ClassDecorator }; }; -export const SkipThrottle = (): MethodDecorator & ClassDecorator => { +export const SkipThrottle = (skip = true): MethodDecorator & ClassDecorator => { return ( target: any, propertyKey?: string | symbol, descriptor?: TypedPropertyDescriptor, ) => { if (descriptor) { - Reflect.defineMetadata(THROTTLER_SKIP, true, descriptor.value); + Reflect.defineMetadata(THROTTLER_SKIP, skip, descriptor.value); return descriptor; } - Reflect.defineMetadata(THROTTLER_SKIP, true, target); + Reflect.defineMetadata(THROTTLER_SKIP, skip, target); return target; }; }; diff --git a/src/throttler.guard.ts b/src/throttler.guard.ts index 5a9c7a69..5d7669b2 100644 --- a/src/throttler.guard.ts +++ b/src/throttler.guard.ts @@ -28,6 +28,11 @@ export class ThrottlerGuard implements CanActivate { const classRef = context.getClass(); const headerPrefix = 'X-RateLimit'; + // Return early if the current route should be skipped. + if (this.reflector.getAllAndOverride(THROTTLER_SKIP, [handler, classRef])) { + return true; + } + // Return early when we have no limit or ttl data. const routeOrClassLimit = this.reflector.getAllAndOverride(THROTTLER_LIMIT, [ handler, @@ -41,14 +46,9 @@ export class ThrottlerGuard implements CanActivate { // Check if specific limits are set at class or route level, otherwise use global options. const limit = routeOrClassLimit || this.options.limit; const ttl = routeOrClassTtl || this.options.ttl; - if (typeof limit === 'undefined' || typeof ttl === 'undefined') { + /* if (typeof limit === 'undefined' || typeof ttl === 'undefined') { return true; - } - - // Return early if the current route should be skipped. - if (this.reflector.getAllAndOverride(THROTTLER_SKIP, [handler, classRef])) { - return true; - } + } */ // Here we start to check the amount of requests being done against the ttl. const req = context.switchToHttp().getRequest(); @@ -73,22 +73,4 @@ export class ThrottlerGuard implements CanActivate { this.storageService.addRecord(key, ttl); return true; } - - normalizeRoutes(routes: Array): RouteInfoRegex[] { - if (!Array.isArray(routes)) return []; - - return routes.map( - (routeObj: string | RouteInfo): RouteInfoRegex => { - const route = - typeof routeObj === 'string' - ? { - path: routeObj, - method: RequestMethod.ALL, - } - : routeObj; - - return { ...route, regex: pathToRegexp(route.path) }; - }, - ); - } } diff --git a/test/app.e2e-spec.ts b/test/app.e2e-spec.ts index ea19a295..18b37157 100644 --- a/test/app.e2e-spec.ts +++ b/test/app.e2e-spec.ts @@ -1,28 +1,22 @@ -import { INestApplication, RequestMethod } from '@nestjs/common'; +import { INestApplication } from '@nestjs/common'; import { AbstractHttpAdapter } from '@nestjs/core'; import { ExpressAdapter } from '@nestjs/platform-express'; +import { FastifyAdapter } from '@nestjs/platform-fastify'; import { Test, TestingModule } from '@nestjs/testing'; import { AppModule } from './app/app.module'; import { httPromise } from './utility/httpromise'; -//${new FastifyAdapter()} | ${'Fastify'} describe.each` adapter | adapterName ${new ExpressAdapter()} | ${'Express'} + ${new FastifyAdapter()} | ${'Fastify'} `('$adapterName Throttler', ({ adapter }: { adapter: AbstractHttpAdapter }) => { let app: INestApplication; beforeAll(async () => { const moduleFixture: TestingModule = await Test.createTestingModule({ imports: [ - AppModule.forRoot({ - excludeRoutes: [ - 'ignored', - { path: 'ignored-2', method: RequestMethod.POST }, - { path: 'ignored-3', method: RequestMethod.ALL }, - { path: 'ignored/:foo', method: RequestMethod.GET }, - ], - }), + AppModule.forRoot(), ], }).compile(); @@ -43,17 +37,11 @@ describe.each` /** * Tests for setting `@Throttle()` at the method level and for ignore routes */ - // ${'/ignored'} | ${'GET'} - // ${'/ignored-2'} | ${'POST'} - // ${'/ignored/something'} | ${'GET'} describe('AppController', () => { - it.each` - url | method - ${'/ignored-3'} | ${'PATCH'} - `( - 'ignore $method $url', - async ({ url, method }: { url: string; method: 'GET' | 'POST' | 'PATCH' }) => { - const response = await httPromise(appUrl + url, method, {}); + it( + 'GET /ignored', + async () => { + const response = await httPromise(appUrl + '/ignored'); expect(response.data).toEqual({ ignored: true }); expect(response.headers).not.toMatchObject({ 'x-ratelimit-limit': '2', @@ -63,7 +51,7 @@ describe.each` }, ); it('GET /', async () => { - const response = await httPromise(appUrl + '/', 'GET', {}); + const response = await httPromise(appUrl + '/'); expect(response.data).toEqual({ success: true }); expect(response.headers).toMatchObject({ 'x-ratelimit-limit': '2', diff --git a/test/app/app.module.ts b/test/app/app.module.ts index ad0fc15a..229ad895 100644 --- a/test/app/app.module.ts +++ b/test/app/app.module.ts @@ -12,7 +12,7 @@ export class AppModule { static forRoot(options?: ThrottlerOptions): DynamicModule { return { module: AppModule, - imports: [ThrottlerModule.forRoot(options)], + imports: [ThrottlerModule.forRoot(options || {})], }; } } diff --git a/test/app/main.ts b/test/app/main.ts index 05adfe30..6d7d3ce8 100644 --- a/test/app/main.ts +++ b/test/app/main.ts @@ -4,7 +4,7 @@ import { AppModule } from './app.module'; async function bootstrap() { const app = await NestFactory.create( - AppModule.forRoot({}), + AppModule.forRoot(), // new ExpressAdapter(), new FastifyAdapter(), );