Skip to content

Commit

Permalink
fix: updates tests and resolves comments from pr
Browse files Browse the repository at this point in the history
  • Loading branch information
jmcdo29 committed Jun 1, 2020
1 parent 16d6fac commit ee87e05
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 52 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 3 additions & 3 deletions src/throttle.decorator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<any>,
) => {
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;
};
};
32 changes: 7 additions & 25 deletions src/throttler.guard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<boolean>(THROTTLER_SKIP, [handler, classRef])) {
return true;
}

// Return early when we have no limit or ttl data.
const routeOrClassLimit = this.reflector.getAllAndOverride<number>(THROTTLER_LIMIT, [
handler,
Expand All @@ -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<boolean>(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();
Expand All @@ -73,22 +73,4 @@ export class ThrottlerGuard implements CanActivate {
this.storageService.addRecord(key, ttl);
return true;
}

normalizeRoutes(routes: Array<string | RouteInfo>): 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) };
},
);
}
}
30 changes: 9 additions & 21 deletions test/app.e2e-spec.ts
Original file line number Diff line number Diff line change
@@ -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();

Expand All @@ -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',
Expand All @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion test/app/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export class AppModule {
static forRoot(options?: ThrottlerOptions): DynamicModule {
return {
module: AppModule,
imports: [ThrottlerModule.forRoot(options)],
imports: [ThrottlerModule.forRoot(options || {})],
};
}
}
2 changes: 1 addition & 1 deletion test/app/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
);
Expand Down

0 comments on commit ee87e05

Please sign in to comment.