refactor(chore): fix sonar code smells (#150)

fix sonar code smells related to magic numbers and cognitive complexity

GH-148

.prettierrc

@@ -1,4 +1,5 @@
 {
+  "arrowParens": "avoid",
   "bracketSpacing": false,
   "singleQuote": true,
   "printWidth": 80,

src/__tests__/acceptance/in-memory-store.ts

@@ -15,7 +15,8 @@ export class InMemoryStore implements Store {
 
   constructor(options?: Options) {
     this.hits = {};
-    this.windowMs = options?.windowMs ?? 60000;
+    const DEFAULT_WINDOW_MS = 60000;
+    this.windowMs = options?.windowMs ?? DEFAULT_WINDOW_MS;
     // Then calculate the reset time using that
     this.resetTime = calculateNextResetTime(this.windowMs);
   }

src/__tests__/acceptance/ratelimit-action-acceptance/rate-limiter.acceptance.ts

@@ -2,6 +2,10 @@ import {Client} from '@loopback/testlab';
 import {memoryStore} from '../store.provider';
 import {TestApplication} from './fixtures/application';
 import {setUpApplication} from './helper';
+
+const OK_STATUS_CODE = 200;
+const TOO_MANY_REQS_CODE = 429;
+
 describe('Acceptance Test Cases', () => {
   let app: TestApplication;
   let client: Client;
@@ -15,48 +19,50 @@ describe('Acceptance Test Cases', () => {
 
   after(async () => app.stop());
 
+  const MAX_REQUESTS = 5;
   it('should hit end point when number of requests is less than max requests allowed', async () => {
     //Max request is set to 5 while binding
-    for (let i = 0; i < 4; i++) {
-      await client.get('/test').expect(200);
+    for (let i = 0; i < MAX_REQUESTS; i++) {
+      await client.get('/test').expect(OK_STATUS_CODE);
     }
   });
 
   it('should hit end point when number of requests is equal to max requests allowed', async () => {
     //Max request is set to 5 while binding
-    for (let i = 0; i < 5; i++) {
-      await client.get('/test').expect(200);
+    for (let i = 0; i < MAX_REQUESTS; i++) {
+      await client.get('/test').expect(OK_STATUS_CODE);
     }
   });
 
   it('should give error when number of requests is greater than max requests allowed', async () => {
     //Max request is set to 5 while binding
-    for (let i = 0; i < 5; i++) {
-      await client.get('/test').expect(200);
+    for (let i = 0; i < MAX_REQUESTS; i++) {
+      await client.get('/test').expect(OK_STATUS_CODE);
     }
-    await client.get('/test').expect(429);
+    await client.get('/test').expect(TOO_MANY_REQS_CODE);
   });
 
   it('should overwrite the default behaviour when rate limit decorator is applied', async () => {
     //Max request is set to 1 in decorator
-    await client.get('/testDecorator').expect(200);
-    await client.get('/testDecorator').expect(429);
+    await client.get('/testDecorator').expect(OK_STATUS_CODE);
+    await client.get('/testDecorator').expect(TOO_MANY_REQS_CODE);
   });
 
   it('should throw no error if requests more than max are sent after window resets', async () => {
     //Max request is set to 5 while binding
-    for (let i = 0; i < 5; i++) {
-      await client.get('/test').expect(200);
+    for (let i = 0; i < MAX_REQUESTS; i++) {
+      await client.get('/test').expect(OK_STATUS_CODE);
     }
+    const TIMEOUT = 2000;
     setTimeout(() => {
       client
         .get('/test')
-        .expect(200)
+        .expect(OK_STATUS_CODE)
         .then(
           () => {},
           () => {},
         );
-    }, 2000);
+    }, TIMEOUT);
   });
 
   async function clearStore() {

src/__tests__/acceptance/store.provider.ts

@@ -10,7 +10,8 @@ export class StoreProvider implements Provider<Store> {
     private readonly config?: RateLimitOptions,
   ) {}
   value(): ValueOrPromise<Store> {
-    const windowMs = this.config?.windowMs ?? 60000;
+    const DEFAULT_WINDOW_MS = 60000;
+    const windowMs = this.config?.windowMs ?? DEFAULT_WINDOW_MS;
     memoryStore.setInterval(windowMs);
     return memoryStore;
   }

src/__tests__/acceptance/test.controller.ts

@@ -2,7 +2,6 @@ import {get} from '@loopback/rest';
 import {ratelimit} from '../..';
 
 export class TestController {
-  constructor() {}
   @get('/test', {
     responses: {
       '200': {

src/__tests__/unit/ratelimit-action.provider.unit.ts

@@ -1,11 +1,11 @@
-import {RestApplication, Request, Response} from '@loopback/rest';
-import {RatelimitActionProvider} from '../../providers';
-import * as RateLimit from 'express-rate-limit';
 import {Constructor} from '@loopback/core';
-import sinon from 'sinon';
-import proxyquire from 'proxyquire';
+import {Request, Response, RestApplication} from '@loopback/rest';
 import {expect} from '@loopback/testlab';
+import * as RateLimit from 'express-rate-limit';
 import {IncrementResponse} from 'express-rate-limit';
+import proxyquire from 'proxyquire';
+import sinon from 'sinon';
+import {RatelimitActionProvider} from '../../providers';
 
 describe('Rate Limit action Service', () => {
   let RatelimitActionMockProvider: Constructor<RatelimitActionProvider>;
@@ -45,34 +45,39 @@ describe('Rate Limit action Service', () => {
     it('returns promise if metadata is not enabled', async () => {
       const result = new RatelimitActionProvider(
         () => {
-          return new Promise((resolve, reject) => {
-            resolve(dataStore);
-          });
+          return Promise.resolve(dataStore);
         },
         rateLimitMetadataFalse,
         restApplication,
       ).action({} as Request, {} as Response);
       await expect(result).to.be.fulfilled();
     });
 
+    const EXPECT_TIMEOUT = 5000;
     it('returns promise if metadata is enabled', async () => {
       const result = new RatelimitActionMockProvider(
         dataStore,
         rateLimitMetadataTrue,
         restApplication,
       )
         .action({} as Request, {} as Response)
-        .catch((err) => err.message);
+        .catch(err => err.message);
       await expect(result).to.be.fulfilled();
-    }).timeout(5000);
+    }).timeout(EXPECT_TIMEOUT);
   });
 
   function increment(key: string): IncrementResponse {
     return {totalHits: 0, resetTime: new Date()};
   }
-  function decrement(key: string): void {}
-  function resetKey(key: string): void {}
-  function resetAll(): void {}
+  function decrement(key: string): void {
+    return;
+  }
+  function resetKey(key: string): void {
+    return;
+  }
+  function resetAll(): void {
+    return;
+  }
 
   function setupMockRatelimitAction() {
     const mockExpressRatelimit = sinon.stub().returns({

src/__tests__/unit/ratelimit-datasource.provider.unit.ts

@@ -14,15 +14,13 @@ describe('Rate Limit datasource Service', () => {
       };
       const ratelimitDatasourceProvider = new RatelimitDatasourceProvider(
         () => {
-          return new Promise((resolve) => {
-            resolve({enabled: true});
-          });
+          return Promise.resolve({enabled: true});
         },
         restApplication,
         config,
       ).value();
       let result;
-      await ratelimitDatasourceProvider.then((value) => {
+      await ratelimitDatasourceProvider.then(value => {
         result = value;
       });
       expect(result).to.have.properties(['expiration', 'prefix', 'client']);
@@ -37,15 +35,13 @@ describe('Rate Limit datasource Service', () => {
       };
       const ratelimitDatasourceProvider = new RatelimitDatasourceProvider(
         () => {
-          return new Promise((resolve) => {
-            resolve({enabled: true});
-          });
+          return Promise.resolve({enabled: true});
         },
         restApplication,
         config,
       ).value();
       let result;
-      await ratelimitDatasourceProvider.then((value) => {
+      await ratelimitDatasourceProvider.then(value => {
         result = value;
       });
       expect(result).to.have.properties(['dbOptions', 'expireTimeMs']);
@@ -57,15 +53,13 @@ describe('Rate Limit datasource Service', () => {
       };
       const ratelimitDatasourceProvider = await new RatelimitDatasourceProvider(
         () => {
-          return new Promise((resolve) => {
-            resolve({enabled: true});
-          });
+          return Promise.resolve({enabled: true});
         },
         restApplication,
         config,
       )
         .value()
-        .catch((err) => err.msg);
+        .catch(err => err.msg);
       expect(ratelimitDatasourceProvider).to.eql(undefined);
     });
   });

src/providers/ratelimit-datasource.provider.ts

@@ -1,12 +1,13 @@
 import {CoreBindings, inject, Provider} from '@loopback/core';
 import {Getter, juggler} from '@loopback/repository';
-import {RateLimitMetadata, RateLimitOptions, Store} from '../types';
-import {RateLimitSecurityBindings} from '../keys';
-import RedisStore, {RedisReply} from 'rate-limit-redis';
+import {HttpErrors, RestApplication} from '@loopback/rest';
 import MemcachedStore from 'rate-limit-memcached';
 import MongoStore from 'rate-limit-mongo';
-import {HttpErrors, RestApplication} from '@loopback/rest';
+import RedisStore, {RedisReply} from 'rate-limit-redis';
 import {TextDecoder} from 'util';
+import {RateLimitSecurityBindings} from '../keys';
+import {RateLimitMetadata, RateLimitOptions, Store} from '../types';
+
 const decoder = new TextDecoder('utf-8');
 export class RatelimitDatasourceProvider implements Provider<Store> {
   constructor(
@@ -35,38 +36,40 @@ export class RatelimitDatasourceProvider implements Provider<Store> {
     if (this.config?.type === 'MemcachedStore') {
       const expiration = (opts.windowMs ?? var1 * var2) / var2;
       return new MemcachedStore({client: this.config?.client, expiration});
-    } else if (this.config?.type === 'MongoStore') {
+    }
+    if (this.config?.type === 'MongoStore') {
       const expireTimeMs = (opts.windowMs ?? var1 * var2) / var2;
       return new MongoStore({
         uri: this.config?.uri,
         collectionName: this.config?.collectionName,
         expireTimeMs,
       });
-    } else {
-      const redisDS = (await this.application.get(
-        `datasources.${this.config?.name}`,
-      )) as juggler.DataSource;
-      if (!redisDS?.connector) {
-        throw new HttpErrors.InternalServerError('Invalid Datasource');
-      }
+    }
 
-      return new RedisStore({
-        sendCommand: async (...args: string[]) => {
-          const command = `${args[0]}`;
-          args.splice(0, 1);
-          let res;
-          try {
-            res = await this.executeRedisCommand(redisDS, command, args);
-            if (command.toLocaleLowerCase() === 'script') {
-              res = decoder.decode(res as ArrayBuffer);
-            }
-          } catch (err) {
-            throw new Error(`Could not execute redis command ${err}`);
-          }
-          return res as RedisReply;
-        },
-      });
+    const redisDS = (await this.application.get(
+      `datasources.${this.config?.name}`,
+    )) as juggler.DataSource;
+
+    if (!redisDS?.connector) {
+      throw new HttpErrors.InternalServerError('Invalid Datasource');
     }
+
+    return new RedisStore({
+      sendCommand: async (...args: string[]) => {
+        const command = `${args[0]}`;
+        args.splice(0, 1);
+        let res;
+        try {
+          res = await this.executeRedisCommand(redisDS, command, args);
+          if (command.toLocaleLowerCase() === 'script') {
+            res = decoder.decode(res as ArrayBuffer);
+          }
+        } catch (err) {
+          throw new Error(`Could not execute redis command ${err}`);
+        }
+        return res as RedisReply;
+      },
+    });
   }
 
   // returns promisified execute function

src/release_notes/post-processing.js

@@ -10,17 +10,17 @@ module.exports = async function (data, callback) {
     const commitTitle = commit.title;
     commit.title = commitTitle.substring(0, commitTitle.indexOf('#') - 1);
 
-    commit.messageLines = commit.messageLines.filter((message) => {
+    commit.messageLines = commit.messageLines.filter(message => {
       if (message.indexOf('efs/remotes/origin') === -1) return message;
     });
 
-    commit.messageLines.forEach((message) => {
+    commit.messageLines.forEach(message => {
       commit.issueno = message.includes('GH-')
         ? message.replace('GH-', '').trim()
         : null;
     });
 
-    const issueDesc = await getIssueDesc(commit.issueno).then((res) => {
+    const issueDesc = await getIssueDesc(commit.issueno).then(res => {
       return res;
     });
     commit.issueTitle = issueDesc;
@@ -48,9 +48,9 @@ function getIssueDesc(issueNo) {
       `https://github.com/sourcefuse/loopback4-ratelimiter/issues/${encodeURIComponent(
         issueNo,
       )}`,
-      (res) => {
+      res => {
         res.setEncoding('utf8');
-        res.on('data', (chunk) => {
+        res.on('data', chunk => {
           result = result + chunk;
         });
         res.on('end', () => {
@@ -69,7 +69,7 @@ function getIssueDesc(issueNo) {
         });
       },
     );
-    req.on('error', (e) => {
+    req.on('error', e => {
       reject(e);
     });
     req.end();

src/release_notes/release-notes.js

@@ -53,7 +53,7 @@ async function addAndCommit() {
   await git.push('origin', 'master');
 }
 
-generateReleaseNotes().catch((ex) => {
+generateReleaseNotes().catch(ex => {
   console.error(ex);
   process.exit(1);
 });

src/types.ts

@@ -1,10 +1,10 @@
 import {Request, Response} from '@loopback/rest';
 import {Options} from 'express-rate-limit';
-import {RedisClient} from 'redis';
-import IORedis = require('ioredis');
 import MemcachedStore from 'rate-limit-memcached';
 import MongoStore from 'rate-limit-mongo';
 import RedisStore from 'rate-limit-redis';
+import {RedisClient} from 'redis';
+import IORedis = require('ioredis');
 
 export type RedisClientType = IORedis.Redis | RedisClient;