From 3493a7a26f640fa8f8b314763a320940b7767051 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Fri, 2 Apr 2021 17:38:22 +0200 Subject: [PATCH 1/7] Logging: Improve mediabackend filesystem log message. Signed-off-by: Philip Molares --- src/main.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.ts b/src/main.ts index e9c2bb5cd..324c254f3 100644 --- a/src/main.ts +++ b/src/main.ts @@ -38,7 +38,7 @@ async function bootstrap(): Promise { ); if (mediaConfig.backend.use === BackendType.FILESYSTEM) { logger.log( - `Serving ${mediaConfig.backend.filesystem.uploadPath} under 'uploads/'`, + `Serving the local folder '${mediaConfig.backend.filesystem.uploadPath}' under '/uploads'`, 'AppBootstrap', ); app.useStaticAssets(mediaConfig.backend.filesystem.uploadPath, { From 31b0d797f36db189b17f53af7edc22169a67e401 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Fri, 2 Apr 2021 17:40:44 +0200 Subject: [PATCH 2/7] Logging: Add OpenAPI log messages Let the user know where the OpenAPI docs can be found. Signed-off-by: Philip Molares --- src/main.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/main.ts b/src/main.ts index 324c254f3..98555016c 100644 --- a/src/main.ts +++ b/src/main.ts @@ -25,8 +25,16 @@ async function bootstrap(): Promise { const mediaConfig = configService.get('mediaConfig'); setupPublicApiDocs(app); + logger.log( + `Serving OpenAPI docs for public api under '/apidoc'`, + 'AppBootstrap', + ); if (process.env.NODE_ENV === 'development') { setupPrivateApiDocs(app); + logger.log( + `Serving OpenAPI docs for private api under '/private/apidoc'`, + 'AppBootstrap', + ); } app.useGlobalPipes( From 0269b5e87a3d2223a18890c32aa08ad6d2ec95ca Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Fri, 2 Apr 2021 19:08:30 +0200 Subject: [PATCH 3/7] Logging: Remove NestConsoleLoggerService This is not necessary anymore, because we can inject ConsoleLoggerService directly. Signed-off-by: Philip Molares --- src/logger/logger.module.ts | 5 ++- src/logger/nest-console-logger.service.ts | 39 ----------------------- src/main.ts | 10 +++--- test/private-api/media.e2e-spec.ts | 4 +-- test/public-api/media.e2e-spec.ts | 4 +-- 5 files changed, 12 insertions(+), 50 deletions(-) delete mode 100644 src/logger/nest-console-logger.service.ts diff --git a/src/logger/logger.module.ts b/src/logger/logger.module.ts index dffa5538d..c95758c8f 100644 --- a/src/logger/logger.module.ts +++ b/src/logger/logger.module.ts @@ -6,10 +6,9 @@ import { Module } from '@nestjs/common'; import { ConsoleLoggerService } from './console-logger.service'; -import { NestConsoleLoggerService } from './nest-console-logger.service'; @Module({ - providers: [ConsoleLoggerService, NestConsoleLoggerService], - exports: [ConsoleLoggerService, NestConsoleLoggerService], + providers: [ConsoleLoggerService], + exports: [ConsoleLoggerService], }) export class LoggerModule {} diff --git a/src/logger/nest-console-logger.service.ts b/src/logger/nest-console-logger.service.ts deleted file mode 100644 index 2fe5b62a4..000000000 --- a/src/logger/nest-console-logger.service.ts +++ /dev/null @@ -1,39 +0,0 @@ -/* - * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) - * - * SPDX-License-Identifier: AGPL-3.0-only - */ - -import { Injectable, LoggerService } from '@nestjs/common'; -import { ConsoleLoggerService } from './console-logger.service'; - -Injectable(); - -export class NestConsoleLoggerService implements LoggerService { - private consoleLoggerService = new ConsoleLoggerService(); - - debug(message: unknown, context?: string): void { - this.consoleLoggerService.setContext(context); - this.consoleLoggerService.debug(message); - } - - error(message: unknown, trace?: string, context?: string): void { - this.consoleLoggerService.setContext(context); - this.consoleLoggerService.error(message, trace); - } - - log(message: unknown, context?: string): void { - this.consoleLoggerService.setContext(context); - this.consoleLoggerService.log(message); - } - - verbose(message: unknown, context?: string): void { - this.consoleLoggerService.setContext(context); - this.consoleLoggerService.verbose(message); - } - - warn(message: unknown, context?: string): void { - this.consoleLoggerService.setContext(context); - this.consoleLoggerService.warn(message); - } -} diff --git a/src/main.ts b/src/main.ts index 98555016c..7b5cf6e3f 100644 --- a/src/main.ts +++ b/src/main.ts @@ -4,20 +4,22 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -import { ValidationPipe } from '@nestjs/common'; +import { LogLevel, ValidationPipe } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { NestFactory } from '@nestjs/core'; import { NestExpressApplication } from '@nestjs/platform-express'; import { AppModule } from './app.module'; import { AppConfig } from './config/app.config'; import { MediaConfig } from './config/media.config'; -import { NestConsoleLoggerService } from './logger/nest-console-logger.service'; import { setupPrivateApiDocs, setupPublicApiDocs } from './utils/swagger'; import { BackendType } from './media/backends/backend-type.enum'; +import { ConsoleLoggerService } from './logger/console-logger.service'; async function bootstrap(): Promise { - const app = await NestFactory.create(AppModule); - const logger = await app.resolve(NestConsoleLoggerService); + const app = await NestFactory.create(AppModule, { + logger: ['error', 'warn', 'log'] as LogLevel[], + }); + const logger = await app.resolve(ConsoleLoggerService); logger.log('Switching logger', 'AppBootstrap'); app.useLogger(logger); const configService = app.get(ConfigService); diff --git a/test/private-api/media.e2e-spec.ts b/test/private-api/media.e2e-spec.ts index 2fc5f7c9a..6c993d7fc 100644 --- a/test/private-api/media.e2e-spec.ts +++ b/test/private-api/media.e2e-spec.ts @@ -22,7 +22,6 @@ import customizationConfigMock from '../../src/config/mock/customization.config. import externalConfigMock from '../../src/config/mock/external-services.config.mock'; import { GroupsModule } from '../../src/groups/groups.module'; import { LoggerModule } from '../../src/logger/logger.module'; -import { NestConsoleLoggerService } from '../../src/logger/nest-console-logger.service'; import { MediaModule } from '../../src/media/media.module'; import { NotesModule } from '../../src/notes/notes.module'; import { NotesService } from '../../src/notes/notes.service'; @@ -31,6 +30,7 @@ import { AuthModule } from '../../src/auth/auth.module'; import { join } from 'path'; import { PrivateApiModule } from '../../src/api/private/private-api.module'; import { UsersService } from '../../src/users/users.service'; +import { ConsoleLoggerService } from '../../src/logger/console-logger.service'; describe('Media', () => { let app: NestExpressApplication; @@ -72,7 +72,7 @@ describe('Media', () => { prefix: '/uploads', }); await app.init(); - const logger = await app.resolve(NestConsoleLoggerService); + const logger = await app.resolve(ConsoleLoggerService); logger.log('Switching logger', 'AppBootstrap'); app.useLogger(logger); const notesService: NotesService = moduleRef.get('NotesService'); diff --git a/test/public-api/media.e2e-spec.ts b/test/public-api/media.e2e-spec.ts index 97d35eb86..40fdebd4c 100644 --- a/test/public-api/media.e2e-spec.ts +++ b/test/public-api/media.e2e-spec.ts @@ -20,7 +20,6 @@ import mediaConfigMock from '../../src/config/mock/media.config.mock'; import appConfigMock from '../../src/config/mock/app.config.mock'; import { GroupsModule } from '../../src/groups/groups.module'; import { LoggerModule } from '../../src/logger/logger.module'; -import { NestConsoleLoggerService } from '../../src/logger/nest-console-logger.service'; import { MediaModule } from '../../src/media/media.module'; import { MediaService } from '../../src/media/media.service'; import { NotesModule } from '../../src/notes/notes.module'; @@ -30,6 +29,7 @@ import { AuthModule } from '../../src/auth/auth.module'; import { TokenAuthGuard } from '../../src/auth/token-auth.guard'; import { MockAuthGuard } from '../../src/auth/mock-auth.guard'; import { join } from 'path'; +import { ConsoleLoggerService } from '../../src/logger/console-logger.service'; describe('Media', () => { let app: NestExpressApplication; @@ -69,7 +69,7 @@ describe('Media', () => { prefix: '/uploads', }); await app.init(); - const logger = await app.resolve(NestConsoleLoggerService); + const logger = await app.resolve(ConsoleLoggerService); logger.log('Switching logger', 'AppBootstrap'); app.useLogger(logger); const notesService: NotesService = moduleRef.get('NotesService'); From a039b85ff4924581f1e482faab47e43b5ad673b9 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 18 Apr 2021 12:29:15 +0200 Subject: [PATCH 4/7] Utils: Add needToLog function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This functions makes it possible to make a partial order of the Loglevel enum. This simplifies the if statements in ConsoleLogger. This is done, because the Loglevel enum already has a string backing for easy conversion from the config environmental variables and therefore can't also have a ordinal number assigned… Signed-off-by: Philip Molares --- src/config/utils.spec.ts | 44 ++++++++++++++++++++++++++++++++++++++++ src/config/utils.ts | 26 ++++++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/src/config/utils.spec.ts b/src/config/utils.spec.ts index 72d5196a5..4471fd48e 100644 --- a/src/config/utils.spec.ts +++ b/src/config/utils.spec.ts @@ -11,9 +11,11 @@ @typescript-eslint/require-await */ import { + needToLog, replaceAuthErrorsWithEnvironmentVariables, toArrayConfig, } from './utils'; +import { Loglevel } from './loglevel.enum'; describe('config utils', () => { describe('toArrayConfig', () => { @@ -46,4 +48,46 @@ describe('config utils', () => { ).toEqual('"HD_AUTH_GITLAB_test_SCOPE'); }); }); + describe('needToLog', () => { + it('currentLevel ERROR', () => { + const currentLevel = Loglevel.ERROR; + expect(needToLog(currentLevel, Loglevel.ERROR)).toBeTruthy(); + expect(needToLog(currentLevel, Loglevel.WARN)).toBeFalsy(); + expect(needToLog(currentLevel, Loglevel.INFO)).toBeFalsy(); + expect(needToLog(currentLevel, Loglevel.DEBUG)).toBeFalsy(); + expect(needToLog(currentLevel, Loglevel.TRACE)).toBeFalsy(); + }); + it('currentLevel WARN', () => { + const currentLevel = Loglevel.WARN; + expect(needToLog(currentLevel, Loglevel.ERROR)).toBeTruthy(); + expect(needToLog(currentLevel, Loglevel.WARN)).toBeTruthy(); + expect(needToLog(currentLevel, Loglevel.INFO)).toBeFalsy(); + expect(needToLog(currentLevel, Loglevel.DEBUG)).toBeFalsy(); + expect(needToLog(currentLevel, Loglevel.TRACE)).toBeFalsy(); + }); + it('currentLevel INFO', () => { + const currentLevel = Loglevel.INFO; + expect(needToLog(currentLevel, Loglevel.ERROR)).toBeTruthy(); + expect(needToLog(currentLevel, Loglevel.WARN)).toBeTruthy(); + expect(needToLog(currentLevel, Loglevel.INFO)).toBeTruthy(); + expect(needToLog(currentLevel, Loglevel.DEBUG)).toBeFalsy(); + expect(needToLog(currentLevel, Loglevel.TRACE)).toBeFalsy(); + }); + it('currentLevel DEBUG', () => { + const currentLevel = Loglevel.DEBUG; + expect(needToLog(currentLevel, Loglevel.ERROR)).toBeTruthy(); + expect(needToLog(currentLevel, Loglevel.WARN)).toBeTruthy(); + expect(needToLog(currentLevel, Loglevel.INFO)).toBeTruthy(); + expect(needToLog(currentLevel, Loglevel.DEBUG)).toBeTruthy(); + expect(needToLog(currentLevel, Loglevel.TRACE)).toBeFalsy(); + }); + it('currentLevel TRACE', () => { + const currentLevel = Loglevel.TRACE; + expect(needToLog(currentLevel, Loglevel.ERROR)).toBeTruthy(); + expect(needToLog(currentLevel, Loglevel.WARN)).toBeTruthy(); + expect(needToLog(currentLevel, Loglevel.INFO)).toBeTruthy(); + expect(needToLog(currentLevel, Loglevel.DEBUG)).toBeTruthy(); + expect(needToLog(currentLevel, Loglevel.TRACE)).toBeTruthy(); + }); + }); }); diff --git a/src/config/utils.ts b/src/config/utils.ts index 50a695f6e..e02ca8ded 100644 --- a/src/config/utils.ts +++ b/src/config/utils.ts @@ -4,6 +4,8 @@ * SPDX-License-Identifier: AGPL-3.0-only */ +import { Loglevel } from './loglevel.enum'; + export function toArrayConfig(configValue: string, separator = ','): string[] { if (!configValue) { return []; @@ -87,3 +89,27 @@ export function replaceAuthErrorsWithEnvironmentVariables( message = message.replace('.accessRole', '_ACCESS_ROLE'); return message; } + +export function needToLog( + currentLoglevel: Loglevel, + requestedLoglevel: Loglevel, +): boolean { + const current = transformLoglevelToInt(currentLoglevel); + const requested = transformLoglevelToInt(requestedLoglevel); + return current >= requested; +} + +function transformLoglevelToInt(loglevel: Loglevel): number { + switch (loglevel) { + case Loglevel.TRACE: + return 5; + case Loglevel.DEBUG: + return 4; + case Loglevel.INFO: + return 3; + case Loglevel.WARN: + return 2; + case Loglevel.ERROR: + return 1; + } +} From 327206d60c4fc070c13ce4c46d8199e533af4586 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Tue, 13 Apr 2021 23:13:47 +0200 Subject: [PATCH 5/7] Logging: Add LogLevels to ConsoleLoggerService Signed-off-by: Philip Molares --- src/logger/console-logger.service.ts | 71 +++++++++++++++++----------- 1 file changed, 43 insertions(+), 28 deletions(-) diff --git a/src/logger/console-logger.service.ts b/src/logger/console-logger.service.ts index aeb1af10b..135018df6 100644 --- a/src/logger/console-logger.service.ts +++ b/src/logger/console-logger.service.ts @@ -4,17 +4,24 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -import { Injectable, Optional, Scope } from '@nestjs/common'; +import { Inject, Injectable, Optional, Scope } from '@nestjs/common'; import { isObject } from '@nestjs/common/utils/shared.utils'; import clc = require('cli-color'); import DateTimeFormatOptions = Intl.DateTimeFormatOptions; +import appConfiguration, { AppConfig } from '../config/app.config'; +import { Loglevel } from '../config/loglevel.enum'; +import { needToLog } from '../config/utils'; @Injectable({ scope: Scope.TRANSIENT }) export class ConsoleLoggerService { private classContext: string; private lastTimestamp: number; - constructor(@Optional() context?: string) { + constructor( + @Inject(appConfiguration.KEY) + private appConfig: AppConfig, + @Optional() context?: string, + ) { this.classContext = context; } @@ -29,43 +36,51 @@ export class ConsoleLoggerService { this.makeContextString(functionContext), false, ); - this.printStackTrace(trace); + ConsoleLoggerService.printStackTrace(trace); } log(message: unknown, functionContext?: string): void { - this.printMessage( - message, - clc.green, - this.makeContextString(functionContext), - false, - ); + if (needToLog(this.appConfig.loglevel, Loglevel.INFO)) { + this.printMessage( + message, + clc.green, + this.makeContextString(functionContext), + false, + ); + } } warn(message: unknown, functionContext?: string): void { - this.printMessage( - message, - clc.yellow, - this.makeContextString(functionContext), - false, - ); + if (needToLog(this.appConfig.loglevel, Loglevel.WARN)) { + this.printMessage( + message, + clc.yellow, + this.makeContextString(functionContext), + false, + ); + } } debug(message: unknown, functionContext?: string): void { - this.printMessage( - message, - clc.magentaBright, - this.makeContextString(functionContext), - false, - ); + if (needToLog(this.appConfig.loglevel, Loglevel.DEBUG)) { + this.printMessage( + message, + clc.magentaBright, + this.makeContextString(functionContext), + false, + ); + } } verbose(message: unknown, functionContext?: string): void { - this.printMessage( - message, - clc.cyanBright, - this.makeContextString(functionContext), - false, - ); + if (needToLog(this.appConfig.loglevel, Loglevel.TRACE)) { + this.printMessage( + message, + clc.cyanBright, + this.makeContextString(functionContext), + false, + ); + } } private makeContextString(functionContext: string): string { @@ -117,7 +132,7 @@ export class ConsoleLoggerService { return result; } - private printStackTrace(trace: string): void { + private static printStackTrace(trace: string): void { if (!trace) { return; } From a87408009d6321070e4776e1eeb55fe09b154b75 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Tue, 13 Apr 2021 23:21:55 +0200 Subject: [PATCH 6/7] Logging: Handle calls with 'undefined' context Signed-off-by: Philip Molares --- src/logger/console-logger.service.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/logger/console-logger.service.ts b/src/logger/console-logger.service.ts index 135018df6..cc8e6a392 100644 --- a/src/logger/console-logger.service.ts +++ b/src/logger/console-logger.service.ts @@ -85,6 +85,9 @@ export class ConsoleLoggerService { private makeContextString(functionContext: string): string { let context = this.classContext; + if (!context) { + context = 'HedgeDoc'; + } if (functionContext) { context += '.' + functionContext + '()'; } From 0ef9a338f3a50d335a9a37c38994262b75daf140 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Wed, 14 Apr 2021 00:19:09 +0200 Subject: [PATCH 7/7] UnitTests: Add appConfigMock This is necessary as the Logger needs this config for the loglevel. Signed-off-by: Philip Molares --- src/api/private/tokens/tokens.controller.spec.ts | 11 ++++++++++- src/auth/auth.service.spec.ts | 12 +++++++++++- src/groups/groups.service.spec.ts | 10 +++++++++- src/users/users.service.spec.ts | 10 +++++++++- 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/api/private/tokens/tokens.controller.spec.ts b/src/api/private/tokens/tokens.controller.spec.ts index 287a5f0a2..2cd958475 100644 --- a/src/api/private/tokens/tokens.controller.spec.ts +++ b/src/api/private/tokens/tokens.controller.spec.ts @@ -12,6 +12,8 @@ import { Identity } from '../../../users/identity.entity'; import { User } from '../../../users/user.entity'; import { AuthToken } from '../../../auth/auth-token.entity'; import { AuthModule } from '../../../auth/auth.module'; +import { ConfigModule } from '@nestjs/config'; +import appConfigMock from '../../../config/mock/app.config.mock'; describe('TokensController', () => { let controller: TokensController; @@ -19,7 +21,14 @@ describe('TokensController', () => { beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ controllers: [TokensController], - imports: [LoggerModule, AuthModule], + imports: [ + ConfigModule.forRoot({ + isGlobal: true, + load: [appConfigMock], + }), + LoggerModule, + AuthModule, + ], }) .overrideProvider(getRepositoryToken(User)) .useValue({}) diff --git a/src/auth/auth.service.spec.ts b/src/auth/auth.service.spec.ts index 0aaa41ae3..844eb94a1 100644 --- a/src/auth/auth.service.spec.ts +++ b/src/auth/auth.service.spec.ts @@ -21,6 +21,8 @@ import { LoggerModule } from '../logger/logger.module'; import { AuthToken } from './auth-token.entity'; import { NotInDBError, TokenNotValidError } from '../errors/errors'; import { Repository } from 'typeorm'; +import { ConfigModule } from '@nestjs/config'; +import appConfigMock from '../config/mock/app.config.mock'; describe('AuthService', () => { let service: AuthService; @@ -38,7 +40,15 @@ describe('AuthService', () => { useClass: Repository, }, ], - imports: [PassportModule, UsersModule, LoggerModule], + imports: [ + ConfigModule.forRoot({ + isGlobal: true, + load: [appConfigMock], + }), + PassportModule, + UsersModule, + LoggerModule, + ], }) .overrideProvider(getRepositoryToken(Identity)) .useValue({}) diff --git a/src/groups/groups.service.spec.ts b/src/groups/groups.service.spec.ts index 840fd2029..96972c77d 100644 --- a/src/groups/groups.service.spec.ts +++ b/src/groups/groups.service.spec.ts @@ -11,6 +11,8 @@ import { Repository } from 'typeorm'; import { Group } from './group.entity'; import { NotInDBError } from '../errors/errors'; import { LoggerModule } from '../logger/logger.module'; +import { ConfigModule } from '@nestjs/config'; +import appConfigMock from '../config/mock/app.config.mock'; describe('GroupsService', () => { let service: GroupsService; @@ -26,7 +28,13 @@ describe('GroupsService', () => { useClass: Repository, }, ], - imports: [LoggerModule], + imports: [ + ConfigModule.forRoot({ + isGlobal: true, + load: [appConfigMock], + }), + LoggerModule, + ], }).compile(); service = module.get(GroupsService); diff --git a/src/users/users.service.spec.ts b/src/users/users.service.spec.ts index 407114f23..4bff57b39 100644 --- a/src/users/users.service.spec.ts +++ b/src/users/users.service.spec.ts @@ -9,6 +9,8 @@ import { getRepositoryToken } from '@nestjs/typeorm'; import { LoggerModule } from '../logger/logger.module'; import { User } from './user.entity'; import { UsersService } from './users.service'; +import { ConfigModule } from '@nestjs/config'; +import appConfigMock from '../config/mock/app.config.mock'; describe('UsersService', () => { let service: UsersService; @@ -22,7 +24,13 @@ describe('UsersService', () => { useValue: {}, }, ], - imports: [LoggerModule], + imports: [ + ConfigModule.forRoot({ + isGlobal: true, + load: [appConfigMock], + }), + LoggerModule, + ], }) .overrideProvider(getRepositoryToken(User)) .useValue({})