Merge pull request #93 from monstar-lab-oss/sivan/logger-service-final

yashmurty · web-flow · commit 6703cf8a9ab8 · 2021-03-09T17:08:26.000+09:00
feat: logger service funtions
diff --git a/src/app.controller.spec.ts b/src/app.controller.spec.ts
@@ -6,7 +6,7 @@ import { RequestContext } from './shared/request-context/request-context.dto';
 
 describe('AppController', () => {
   let moduleRef: TestingModule;
-  const mockedLogger = { setContext: jest.fn(), logWithContext: jest.fn() };
+  const mockedLogger = { setContext: jest.fn(), log: jest.fn() };
 
   beforeEach(async () => {
     moduleRef = await Test.createTestingModule({
diff --git a/src/app.controller.ts b/src/app.controller.ts
@@ -15,7 +15,7 @@ export class AppController {
 
   @Get()
   getHello(@ReqContext() ctx: RequestContext): string {
-    this.logger.logWithContext(ctx, 'Hello world from App controller');
+    this.logger.log(ctx, 'Hello world from App controller');
 
     return this.appService.getHello(ctx);
   }
diff --git a/src/app.service.ts b/src/app.service.ts
@@ -9,7 +9,7 @@ export class AppService {
   }
 
   getHello(ctx: RequestContext): string {
-    this.logger.logWithContext(ctx, 'Hello world from App service');
+    this.logger.log(ctx, 'Hello world from App service');
 
     return 'Hello World!';
   }
diff --git a/src/auth/controllers/auth.controller.spec.ts b/src/auth/controllers/auth.controller.spec.ts
@@ -19,7 +19,7 @@ describe('AuthController', () => {
     refreshToken: jest.fn(),
   };
 
-  const mockedLogger = { setContext: jest.fn(), logWithContext: jest.fn() };
+  const mockedLogger = { setContext: jest.fn(), log: jest.fn() };
 
   beforeEach(async () => {
     moduleRef = await Test.createTestingModule({
diff --git a/src/auth/controllers/auth.controller.ts b/src/auth/controllers/auth.controller.ts
@@ -56,7 +56,7 @@ export class AuthController {
     // eslint-disable-next-line @typescript-eslint/no-unused-vars
     @Body() credential: LoginInput,
   ): BaseApiResponse<AuthTokenOutput> {
-    this.logger.logWithContext(ctx, `${this.login.name} was called`);
+    this.logger.log(ctx, `${this.login.name} was called`);
 
     const authToken = this.authService.login(ctx);
     return { data: authToken, meta: {} };
@@ -98,7 +98,7 @@ export class AuthController {
     // eslint-disable-next-line @typescript-eslint/no-unused-vars
     @Body() credential: RefreshTokenInput,
   ): Promise<BaseApiResponse<AuthTokenOutput>> {
-    this.logger.logWithContext(ctx, `${this.refreshToken.name} was called`);
+    this.logger.log(ctx, `${this.refreshToken.name} was called`);
 
     const authToken = await this.authService.refreshToken(ctx);
     return { data: authToken, meta: {} };
diff --git a/src/auth/services/auth.service.spec.ts b/src/auth/services/auth.service.spec.ts
@@ -63,7 +63,7 @@ describe('AuthService', () => {
 
   const mockedConfigService = { get: jest.fn() };
 
-  const mockedLogger = { setContext: jest.fn(), logWithContext: jest.fn() };
+  const mockedLogger = { setContext: jest.fn(), log: jest.fn() };
 
   beforeEach(async () => {
     const moduleRef: TestingModule = await Test.createTestingModule({
diff --git a/src/auth/services/auth.service.ts b/src/auth/services/auth.service.ts
@@ -31,7 +31,7 @@ export class AuthService {
     username: string,
     pass: string,
   ): Promise<UserAccessTokenClaims> {
-    this.logger.logWithContext(ctx, `${this.validateUser.name} was called`);
+    this.logger.log(ctx, `${this.validateUser.name} was called`);
 
     // The userService will throw Unauthorized in case of invalid username/password.
     const user = await this.userService.validateUsernamePassword(
@@ -49,7 +49,7 @@ export class AuthService {
   }
 
   login(ctx: RequestContext): AuthTokenOutput {
-    this.logger.logWithContext(ctx, `${this.login.name} was called`);
+    this.logger.log(ctx, `${this.login.name} was called`);
 
     return this.getAuthToken(ctx, ctx.user);
   }
@@ -58,7 +58,7 @@ export class AuthService {
     ctx: RequestContext,
     input: RegisterInput,
   ): Promise<RegisterOutput> {
-    this.logger.logWithContext(ctx, `${this.register.name} was called`);
+    this.logger.log(ctx, `${this.register.name} was called`);
 
     // TODO : Setting default role as USER here. Will add option to change this later via ADMIN users.
     input.roles = [ROLE.USER];
@@ -71,7 +71,7 @@ export class AuthService {
   }
 
   async refreshToken(ctx: RequestContext): Promise<AuthTokenOutput> {
-    this.logger.logWithContext(ctx, `${this.refreshToken.name} was called`);
+    this.logger.log(ctx, `${this.refreshToken.name} was called`);
 
     const user = await this.userService.findById(ctx, ctx.user.id);
     if (!user) {
@@ -85,7 +85,7 @@ export class AuthService {
     ctx: RequestContext,
     user: UserAccessTokenClaims | UserOutput,
   ): AuthTokenOutput {
-    this.logger.logWithContext(ctx, `${this.getAuthToken.name} was called`);
+    this.logger.log(ctx, `${this.getAuthToken.name} was called`);
 
     const subject = { sub: user.id };
     const payload = {
diff --git a/src/auth/strategies/local.strategy.ts b/src/auth/strategies/local.strategy.ts
@@ -31,7 +31,7 @@ export class LocalStrategy extends PassportStrategy(Strategy, STRATEGY_LOCAL) {
   ): Promise<UserAccessTokenClaims> {
     const ctx = createRequestContext(request);
 
-    this.logger.logWithContext(ctx, `${this.validate.name} was called`);
+    this.logger.log(ctx, `${this.validate.name} was called`);
 
     const user = await this.authService.validateUser(ctx, username, password);
     // Passport automatically creates a user object, based on the value we return from the validate() method,
diff --git a/src/cli.ts b/src/cli.ts
@@ -3,7 +3,6 @@ import { ConfigService } from '@nestjs/config';
 
 import { AppModule } from './app.module';
 
-import { AppLogger } from './shared/logger/logger.service';
 import { UserService } from './user/services/user.service';
 import { ROLE } from './auth/constants/role.constant';
 import { CreateUserInput } from './user/dtos/user-create-input.dto';
@@ -12,8 +11,6 @@ import { RequestContext } from './shared/request-context/request-context.dto';
 async function bootstrap() {
   const app = await NestFactory.createApplicationContext(AppModule);
 
-  app.useLogger(new AppLogger());
-
   const configService = app.get(ConfigService);
   const defaultAdminUserPassword = configService.get<string>(
     'defaultAdminUserPassword',
diff --git a/src/main.ts b/src/main.ts
@@ -4,14 +4,12 @@ import { ValidationPipe } from '@nestjs/common';
 import { SwaggerModule, DocumentBuilder } from '@nestjs/swagger';
 
 import { AppModule } from './app.module';
-import { AppLogger } from './shared/logger/logger.service';
 import { RequestIdMiddleware } from './shared/middlewares/request-id/request-id.middleware';
 
 async function bootstrap() {
   const app = await NestFactory.create(AppModule);
   app.setGlobalPrefix('api/v1');
 
-  app.useLogger(new AppLogger());
   app.useGlobalPipes(new ValidationPipe({ transform: true, whitelist: true }));
   app.use(RequestIdMiddleware);
   app.enableCors();
diff --git a/src/shared/filters/all-exceptions.filter.spec.ts b/src/shared/filters/all-exceptions.filter.spec.ts
@@ -25,7 +25,6 @@ describe('AllExceptionsFilter', () => {
   const mockedLogger = {
     warn: jest.fn().mockReturnThis(),
     setContext: jest.fn().mockReturnThis(),
-    warnWithContext: jest.fn().mockReturnThis(),
   };
   let filter: AllExceptionsFilter<any>;
 
diff --git a/src/shared/filters/all-exceptions.filter.ts b/src/shared/filters/all-exceptions.filter.ts
@@ -77,7 +77,7 @@ export class AllExceptionsFilter<T> implements ExceptionFilter {
       requestId,
       timestamp,
     };
-    this.logger.warnWithContext(requestContext, error.message, {
+    this.logger.warn(requestContext, error.message, {
       error,
       stack,
     });
diff --git a/src/shared/interceptors/logging.interceptor.ts b/src/shared/interceptors/logging.interceptor.ts
@@ -30,7 +30,7 @@ export class LoggingInterceptor implements NestInterceptor {
 
         const resData = { method, statusCode, responseTime };
 
-        this.appLogger.logWithContext(ctx, 'Request completed', { resData });
+        this.appLogger.log(ctx, 'Request completed', { resData });
       }),
     );
   }
diff --git a/src/shared/logger/logger.service.ts b/src/shared/logger/logger.service.ts
@@ -1,9 +1,9 @@
-import { Injectable, LoggerService, Scope } from '@nestjs/common';
+import { Injectable, Scope } from '@nestjs/common';
 import { createLogger, Logger, transports } from 'winston';
 import { RequestContext } from '../request-context/request-context.dto';
 
 @Injectable({ scope: Scope.TRANSIENT })
-export class AppLogger implements LoggerService {
+export class AppLogger {
   private context?: string;
   private logger: Logger;
 
@@ -17,53 +17,49 @@ export class AppLogger implements LoggerService {
     });
   }
 
-  // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
-  log(message: any, context?: string): Logger {
-    return this.logger.info(message, {
-      context: context || this.context,
-    });
-  }
-
-  // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
-  error(message: any, trace?: string, context?: string): Logger {
-    return this.logger.error(message, {
-      trace,
-      context: context || this.context,
-    });
-  }
+  error(
+    ctx: RequestContext,
+    // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
+    message: string,
+    meta?: Record<string, any>,
+  ): Logger {
+    const timestamp = new Date().toISOString();
 
-  // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
-  warn(message: any, context?: string): Logger {
-    return this.logger.warn(message, {
-      context: context || this.context,
+    return this.logger.error({
+      message,
+      contextName: this.context,
+      ctx,
+      timestamp,
+      ...meta,
     });
   }
 
-  // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
-  debug(message: any, context?: string): Logger {
-    return this.logger.debug(message, {
-      context: context || this.context,
-    });
-  }
+  warn(
+    ctx: RequestContext,
+    // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
+    message: string,
+    meta?: Record<string, any>,
+  ): Logger {
+    const timestamp = new Date().toISOString();
 
-  // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
-  verbose(message: any, context?: string): Logger {
-    return this.logger.verbose(message, {
-      context: context || this.context,
+    return this.logger.warn({
+      message,
+      contextName: this.context,
+      ctx,
+      timestamp,
+      ...meta,
     });
   }
 
-  // TODO : This is a temporary function, it will be renamed to `log` once we update it across
-  // all controllers, services, etc.
-  logWithContext(
+  debug(
     ctx: RequestContext,
     // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
-    message: any,
+    message: string,
     meta?: Record<string, any>,
   ): Logger {
     const timestamp = new Date().toISOString();
 
-    return this.logger.info({
+    return this.logger.debug({
       message,
       contextName: this.context,
       ctx,
@@ -72,15 +68,16 @@ export class AppLogger implements LoggerService {
     });
   }
 
-  warnWithContext(
+  // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
+  verbose(
     ctx: RequestContext,
     // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
-    message: any,
+    message: string,
     meta?: Record<string, any>,
   ): Logger {
     const timestamp = new Date().toISOString();
 
-    return this.logger.warn({
+    return this.logger.verbose({
       message,
       contextName: this.context,
       ctx,
@@ -89,15 +86,15 @@ export class AppLogger implements LoggerService {
     });
   }
 
-  errorWithContext(
+  log(
     ctx: RequestContext,
     // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
-    message: any,
+    message: string,
     meta?: Record<string, any>,
   ): Logger {
     const timestamp = new Date().toISOString();
 
-    return this.logger.error({
+    return this.logger.info({
       message,
       contextName: this.context,
       ctx,
diff --git a/src/user/controllers/user.controller.spec.ts b/src/user/controllers/user.controller.spec.ts
@@ -19,7 +19,7 @@ describe('UserController', () => {
     updateUser: jest.fn(),
   };
 
-  const mockedLogger = { setContext: jest.fn(), logWithContext: jest.fn() };
+  const mockedLogger = { setContext: jest.fn(), log: jest.fn() };
 
   beforeEach(async () => {
     const moduleRef: TestingModule = await Test.createTestingModule({
diff --git a/src/user/controllers/user.controller.ts b/src/user/controllers/user.controller.ts
@@ -56,7 +56,7 @@ export class UserController {
   async getMyProfile(
     @ReqContext() ctx: RequestContext,
   ): Promise<BaseApiResponse<UserOutput>> {
-    this.logger.logWithContext(ctx, `${this.getMyProfile.name} was called`);
+    this.logger.log(ctx, `${this.getMyProfile.name} was called`);
 
     const user = await this.userService.findById(ctx, ctx.user.id);
     return { data: user, meta: {} };
@@ -81,7 +81,7 @@ export class UserController {
     @ReqContext() ctx: RequestContext,
     @Query() query: PaginationParamsDto,
   ): Promise<BaseApiResponse<UserOutput[]>> {
-    this.logger.logWithContext(ctx, `${this.getUsers.name} was called`);
+    this.logger.log(ctx, `${this.getUsers.name} was called`);
 
     const { users, count } = await this.userService.getUsers(
       ctx,
@@ -111,7 +111,7 @@ export class UserController {
     @ReqContext() ctx: RequestContext,
     @Param('id') id: number,
   ): Promise<BaseApiResponse<UserOutput>> {
-    this.logger.logWithContext(ctx, `${this.getUser.name} was called`);
+    this.logger.log(ctx, `${this.getUser.name} was called`);
 
     const user = await this.userService.getUserById(ctx, id);
     return { data: user, meta: {} };
@@ -137,7 +137,7 @@ export class UserController {
     @Param('id') userId: number,
     @Body() input: UpdateUserInput,
   ): Promise<BaseApiResponse<UserOutput>> {
-    this.logger.logWithContext(ctx, `${this.updateUser.name} was called`);
+    this.logger.log(ctx, `${this.updateUser.name} was called`);
 
     const user = await this.userService.updateUser(ctx, userId, input);
     return { data: user, meta: {} };
diff --git a/src/user/services/user.service.spec.ts b/src/user/services/user.service.spec.ts
@@ -28,7 +28,7 @@ describe('UserService', () => {
     roles: [ROLE.USER],
   };
 
-  const mockedLogger = { setContext: jest.fn(), logWithContext: jest.fn() };
+  const mockedLogger = { setContext: jest.fn(), log: jest.fn() };
 
   beforeEach(async () => {
     const moduleRef: TestingModule = await Test.createTestingModule({
diff --git a/src/user/services/user.service.ts b/src/user/services/user.service.ts

Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@ export class AppController {`
`15`	`15`
`16`	`16`	`@Get()`
`17`	`17`	`getHello(@ReqContext() ctx: RequestContext): string {`
`18`		`- this.logger.logWithContext(ctx, 'Hello world from App controller');`
	`18`	`+ this.logger.log(ctx, 'Hello world from App controller');`
`19`	`19`
`20`	`20`	`return this.appService.getHello(ctx);`
`21`	`21`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@ export class AppService {`
`9`	`9`	`}`
`10`	`10`
`11`	`11`	`getHello(ctx: RequestContext): string {`
`12`		`- this.logger.logWithContext(ctx, 'Hello world from App service');`
	`12`	`+ this.logger.log(ctx, 'Hello world from App service');`
`13`	`13`
`14`	`14`	`return 'Hello World!';`
`15`	`15`	`}`