Skip to content

Commit

Permalink
Merge branch 'main' into EW-777
Browse files Browse the repository at this point in the history
  • Loading branch information
MajedAlaitwniCap authored Dec 23, 2024
2 parents f4d0542 + cb1f7a6 commit 838325f
Show file tree
Hide file tree
Showing 10 changed files with 153 additions and 2 deletions.
3 changes: 3 additions & 0 deletions apps/server/src/apps/admin-api-server.app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { LegacyLogger, Logger } from '@src/core/logger';
import { AdminApiServerModule } from '@modules/server/admin-api.server.module';
import express from 'express';
import { install as sourceMapInstall } from 'source-map-support';
import { createRequestLoggerMiddleware } from './helpers/request-logger-middleware';
import {
AppStartLoggable,
enableOpenApiDocs,
Expand All @@ -23,6 +24,8 @@ async function bootstrap() {
const nestAdminServerApp = await NestFactory.create(AdminApiServerModule, nestAdminServerExpressAdapter);
const logger = await nestAdminServerApp.resolve(Logger);
const legacyLogger = await nestAdminServerApp.resolve(LegacyLogger);
nestAdminServerApp.use(createRequestLoggerMiddleware());

nestAdminServerApp.useLogger(legacyLogger);
nestAdminServerApp.enableCors();

Expand Down
2 changes: 2 additions & 0 deletions apps/server/src/apps/board-collaboration.app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { RedisIoAdapter } from '@infra/socketio';
import { BoardCollaborationModule } from '@modules/board/board-collaboration.app.module';
import express from 'express';
import { ExpressAdapter } from '@nestjs/platform-express';
import { createRequestLoggerMiddleware } from './helpers/request-logger-middleware';
import {
enableOpenApiDocs,
addPrometheusMetricsMiddlewaresIfEnabled,
Expand All @@ -37,6 +38,7 @@ async function bootstrap() {
};
enableOpenApiDocs(nestApp, 'docs', options);
const logger = await nestApp.resolve(Logger);
nestApp.use(createRequestLoggerMiddleware());

await nestApp.init();

Expand Down
2 changes: 2 additions & 0 deletions apps/server/src/apps/common-cartridge.app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { LegacyLogger, Logger } from '@src/core/logger';
import { CommonCartridgeApiModule } from '@modules/common-cartridge/common-cartridge-api.module';
import express from 'express';
import { install as sourceMapInstall } from 'source-map-support';
import { createRequestLoggerMiddleware } from './helpers/request-logger-middleware';
import {
AppStartLoggable,
enableOpenApiDocs,
Expand All @@ -27,6 +28,7 @@ async function bootstrap() {

const rootExpress = express();
const logger = await nestApp.resolve(Logger);
nestApp.use(createRequestLoggerMiddleware());

addPrometheusMetricsMiddlewaresIfEnabled(logger, rootExpress);

Expand Down
2 changes: 2 additions & 0 deletions apps/server/src/apps/files-storage.app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { FilesStorageApiModule } from '@modules/files-storage/files-storage-api.
import { API_VERSION_PATH } from '@modules/files-storage/files-storage.const';
import { SwaggerDocumentOptions } from '@nestjs/swagger';
import { LegacyLogger } from '@src/core/logger';
import { createRequestLoggerMiddleware } from './helpers/request-logger-middleware';
import { enableOpenApiDocs } from './helpers';

async function bootstrap() {

Check warning on line 18 in apps/server/src/apps/files-storage.app.ts

View workflow job for this annotation

GitHub Actions / nest_lint

Missing return type on function
Expand All @@ -28,6 +29,7 @@ async function bootstrap() {

// customize nest app settings
nestApp.enableCors({ exposedHeaders: ['Content-Disposition'] });
nestApp.use(createRequestLoggerMiddleware());

const options: SwaggerDocumentOptions = {
operationIdFactory: (_controllerKey: string, methodKey: string) => methodKey,
Expand Down
2 changes: 2 additions & 0 deletions apps/server/src/apps/fwu-learning-contents.app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { install as sourceMapInstall } from 'source-map-support';
// application imports
import { LegacyLogger } from '@src/core/logger';
import { FwuLearningContentsModule } from '@modules/fwu-learning-contents';
import { createRequestLoggerMiddleware } from './helpers/request-logger-middleware';
import { enableOpenApiDocs } from './helpers';

async function bootstrap() {

Check warning on line 16 in apps/server/src/apps/fwu-learning-contents.app.ts

View workflow job for this annotation

GitHub Actions / nest_lint

Missing return type on function
Expand All @@ -26,6 +27,7 @@ async function bootstrap() {
// customize nest app settings
nestApp.enableCors({ exposedHeaders: ['Content-Disposition'] });
enableOpenApiDocs(nestApp, 'docs');
nestApp.use(createRequestLoggerMiddleware());

await nestApp.init();

Expand Down
3 changes: 3 additions & 0 deletions apps/server/src/apps/h5p-editor.app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { install as sourceMapInstall } from 'source-map-support';
// application imports
import { LegacyLogger } from '@src/core/logger';
import { H5PEditorModule } from '@modules/h5p-editor';
import { createRequestLoggerMiddleware } from './helpers/request-logger-middleware';
import { enableOpenApiDocs } from './helpers';

async function bootstrap() {

Check warning on line 16 in apps/server/src/apps/h5p-editor.app.ts

View workflow job for this annotation

GitHub Actions / nest_lint

Missing return type on function
Expand All @@ -24,6 +25,8 @@ async function bootstrap() {
// WinstonLogger
nestApp.useLogger(await nestApp.resolve(LegacyLogger));

nestApp.use(createRequestLoggerMiddleware());

// customize nest app settings
nestApp.enableCors({ exposedHeaders: ['Content-Disposition'] });
enableOpenApiDocs(nestApp, 'docs');
Expand Down
99 changes: 99 additions & 0 deletions apps/server/src/apps/helpers/request-logger-middleware.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { Configuration } from '@hpi-schul-cloud/commons/lib';
import { Logger } from '@nestjs/common';
import { Request, Response, NextFunction } from 'express';
import { createRequestLoggerMiddleware } from './request-logger-middleware';

jest.mock('@hpi-schul-cloud/commons/lib', () => {
return {
Configuration: {
get: jest.fn(),
},
};
});

describe('RequestLoggerMiddleware', () => {
let mockRequest: Partial<Request>;
let mockResponse: Partial<Response>;
let nextFunction: NextFunction;
let loggerSpy: jest.SpyInstance;
let errorLoggerSpy: jest.SpyInstance;

beforeEach(() => {
mockRequest = {
method: 'GET',
originalUrl: '/test',
};

mockResponse = {
statusCode: 200,
get: jest.fn(),
on: jest.fn(),
};

nextFunction = jest.fn();

loggerSpy = jest.spyOn(Logger.prototype, 'log');
errorLoggerSpy = jest.spyOn(Logger.prototype, 'error');
});

afterEach(() => {
jest.clearAllMocks();
});

it('should call next() when logging is disabled', () => {
jest.spyOn(Configuration, 'get').mockReturnValue(false);

const middleware = createRequestLoggerMiddleware();
middleware(mockRequest as Request, mockResponse as Response, nextFunction);

expect(nextFunction).toHaveBeenCalled();
expect(mockResponse.on).not.toHaveBeenCalled();
});

it('should log request details when logging is enabled', () => {
jest.spyOn(Configuration, 'get').mockReturnValue(true);

jest.spyOn(process, 'hrtime').mockReturnValueOnce([0, 0]);
jest.spyOn(mockResponse, 'get').mockImplementation().mockReturnValue('100');

// eslint-disable-next-line @typescript-eslint/ban-types
let finishCallback: Function | undefined;
// eslint-disable-next-line @typescript-eslint/ban-types
mockResponse.on = jest.fn().mockImplementation((event: string, callback: Function) => {
finishCallback = callback;
});

const middleware = createRequestLoggerMiddleware();
middleware(mockRequest as Request, mockResponse as Response, nextFunction);

expect(nextFunction).toHaveBeenCalled();
expect(mockResponse.on).toHaveBeenCalledWith('finish', expect.any(Function));

// Simulate response finish
jest.spyOn(process, 'hrtime').mockReturnValueOnce([1, 0]);

// Make sure callback was set before calling it
expect(finishCallback).toBeDefined();
finishCallback?.();

expect(loggerSpy).toHaveBeenCalledWith('GET /test 200 1000ms 100');
});

it('should handle errors during logging', () => {
jest.spyOn(Configuration, 'get').mockReturnValue(true);
// eslint-disable-next-line @typescript-eslint/ban-types
mockResponse.on = jest.fn().mockImplementation((event: string, callback: Function) => {
callback();
});

// Force an error by making response.get throw
mockResponse.get = jest.fn().mockImplementation(() => {
throw new Error('Test error');
});

const middleware = createRequestLoggerMiddleware();
middleware(mockRequest as Request, mockResponse as Response, nextFunction);

expect(errorLoggerSpy).toHaveBeenCalledWith('unable to write accesslog', Error('Test error'));
});
});
33 changes: 33 additions & 0 deletions apps/server/src/apps/helpers/request-logger-middleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { Request, Response, NextFunction } from 'express';
import { Configuration } from '@hpi-schul-cloud/commons/lib';
import { Logger } from '@nestjs/common';

export const createRequestLoggerMiddleware = (): ((
request: Request,
response: Response,
next: NextFunction
) => void) => {
const enabled = Configuration.get('REQUEST_LOGGING_ENABLED') as boolean;
const logger = new Logger('REQUEST_LOG');

return (request: Request, response: Response, next: NextFunction): void => {
if (enabled) {
const startAt = process.hrtime();
const { method, originalUrl } = request;

response.on('finish', () => {
try {
const { statusCode } = response;
const contentLength = response.get('content-length') || 'unknown';
const diff = process.hrtime(startAt);
const responseTime = diff[0] * 1e3 + diff[1] * 1e-6;
logger.log(`${method} ${originalUrl} ${statusCode} ${responseTime}ms ${contentLength}`);
} catch (error) {
logger.error('unable to write accesslog', error);
}
});
}

next();
};
};
5 changes: 4 additions & 1 deletion apps/server/src/apps/management.app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,20 @@ import { install as sourceMapInstall } from 'source-map-support';
// application imports
import { LegacyLogger } from '@src/core/logger';
import { ManagementServerModule } from '@modules/management';
import { createRequestLoggerMiddleware } from './helpers/request-logger-middleware';
import { enableOpenApiDocs } from './helpers';

async function bootstrap() {
sourceMapInstall();

// create the NestJS application on a seperate express instance
// create the NestJS application on a separate express instance
const nestExpress = express();

const nestExpressAdapter = new ExpressAdapter(nestExpress);
const nestApp = await NestFactory.create(ManagementServerModule, nestExpressAdapter);

nestApp.use(createRequestLoggerMiddleware());

// WinstonLogger
nestApp.useLogger(await nestApp.resolve(LegacyLogger));

Expand Down
4 changes: 3 additions & 1 deletion apps/server/src/apps/server.app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { join } from 'path';

// register source-map-support for debugging
import { install as sourceMapInstall } from 'source-map-support';
import { createRequestLoggerMiddleware } from './helpers/request-logger-middleware';
import {
AppStartLoggable,
enableOpenApiDocs,
Expand All @@ -34,7 +35,7 @@ import legacyAppPromise = require('../../../../src/app');
async function bootstrap() {
sourceMapInstall();

// create the NestJS application on a seperate express instance
// create the NestJS application on a separate express instance
const nestExpress = express();
const nestExpressAdapter = new ExpressAdapter(nestExpress);
const nestApp = await NestFactory.create(ServerModule, nestExpressAdapter);
Expand All @@ -45,6 +46,7 @@ async function bootstrap() {
nestApp.useLogger(legacyLogger);

const logger = await nestApp.resolve(Logger);
nestApp.use(createRequestLoggerMiddleware());

// load the legacy feathers/express server
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
Expand Down

0 comments on commit 838325f

Please sign in to comment.