diff --git a/packages/server/src/middleware/SessionMiddleware.ts b/packages/server/src/middleware/SessionMiddleware.ts index 3bb6531..9c76670 100644 --- a/packages/server/src/middleware/SessionMiddleware.ts +++ b/packages/server/src/middleware/SessionMiddleware.ts @@ -6,6 +6,7 @@ import {ITransactionManager} from '../database/ITransactionManager' import {Session as SessionEntity} from '../entities/Session' import {SessionStore} from '../session/SessionStore' import {UrlWithStringQuery} from 'url' +import {apiLogger} from '../logger' export interface ISessionOptions { transactionManager: ITransactionManager, @@ -32,7 +33,8 @@ export class SessionMiddleware implements IMiddleware { path: params.baseUrl.path, }, store: new SessionStore({ - cleanup: 1, + cleanupDelay: 60 * 1000, + logger: apiLogger, getRepository: this.getRepository, ttl: 1, buildSession: this.buildSession, diff --git a/packages/server/src/session/SessionStore.test.ts b/packages/server/src/session/SessionStore.test.ts index 52d002d..a8fc691 100644 --- a/packages/server/src/session/SessionStore.test.ts +++ b/packages/server/src/session/SessionStore.test.ts @@ -3,6 +3,7 @@ import request from 'supertest' import {SessionStore} from './SessionStore' import {ISession} from './ISession' import ExpressSession from 'express-session' +import loggerFactory from '@rondo.dev/logger' import { createConnection, Column, Connection, Entity, Index, PrimaryColumn, Repository, @@ -51,7 +52,8 @@ describe('SessionStore', () => { maxAge: 10, }, store: new SessionStore({ - cleanup: 1, + logger: loggerFactory.getLogger('api'), + cleanupDelay: 60 * 1000, getRepository: () => repository, ttl: 1, buildSession: (sd, s) => ({...s, extraData: 'test'}), diff --git a/packages/server/src/session/SessionStore.ts b/packages/server/src/session/SessionStore.ts index 1fafdbb..e0452b1 100644 --- a/packages/server/src/session/SessionStore.ts +++ b/packages/server/src/session/SessionStore.ts @@ -2,6 +2,7 @@ import {Store} from 'express-session' import {ISession} from './ISession' import {Repository, LessThan} from 'typeorm' import {debounce} from '@rondo.dev/tasq' +import { ILogger } from '@rondo.dev/logger' type SessionData = Express.SessionData type Callback = (err?: any, session?: SessionData) => void @@ -9,8 +10,9 @@ type CallbackErr = (err?: any) => void export interface ISessionStoreOptions { readonly ttl: number - readonly cleanup: number + readonly cleanupDelay: number readonly getRepository: TRepositoryFactory + readonly logger: ILogger, buildSession(sessionData: SessionData, session: ISession): S } @@ -24,25 +26,27 @@ export type TRepositoryFactory = () => Repository export class SessionStore extends Store { protected readonly getRepository: TRepositoryFactory - protected readonly cleanup: (...args: never[]) => void + + readonly cleanup = debounce(async () => { + try { + const now = Date.now() + // this method is debounced because is caused deadlock errors in tests. + // Be wary of future problems. Debounce should fix it but this still + // needs to be thorughly tested. The problem is a the delete statement + // which locks the whole table. + await this.getRepository().delete({ + expiredAt: LessThan(now), + } as any) + } catch (err) { + this.options.logger.error('Error cleaning sessions: %s', err.stack) + } + }, 1000) constructor( protected readonly options: ISessionStoreOptions, ) { super() this.getRepository = options.getRepository - - this.cleanup = debounce(async () => { - try { - const now = Date.now() - // FIXME causes deadlocks in tests - await this.getRepository().delete({ - expiredAt: LessThan(now), - } as any) - } catch (err) { - console.log('error cleaning sessions', err) - } - }, 1000) } protected async promiseToCallback( @@ -75,6 +79,8 @@ export class SessionStore extends Store { } set = (sid: string, session: SessionData, callback?: CallbackErr) => { + this.cleanup.cancel() + const promise = Promise.resolve() .then(() => this.saveSession( this.options.buildSession(session, { diff --git a/packages/server/src/test-utils/TestUtils.ts b/packages/server/src/test-utils/TestUtils.ts index 30a526a..2e960c9 100644 --- a/packages/server/src/test-utils/TestUtils.ts +++ b/packages/server/src/test-utils/TestUtils.ts @@ -38,10 +38,13 @@ export class TestUtils { let context: any + beforeAll(async () => { + connection = await database.connect() + }) + beforeEach(async () => { context = namespace.createContext(); (namespace as any).enter(context) - connection = await database.connect() queryRunner = connection.createQueryRunner() await queryRunner.connect() namespace.set(TRANSACTION_ID, shortid()) @@ -56,10 +59,13 @@ export class TestUtils { } await queryRunner.release() namespace.set(TRANSACTION_ID, undefined) - namespace.set(ENTITY_MANAGER, undefined) - await connection.close(); + namespace.set(ENTITY_MANAGER, undefined); (namespace as any).exit(context) }) + + afterAll(async () => { + await database.close() + }) } async createRole(name: string) { diff --git a/packages/tasq/src/debounce.ts b/packages/tasq/src/debounce.ts index b50e2f9..affa369 100644 --- a/packages/tasq/src/debounce.ts +++ b/packages/tasq/src/debounce.ts @@ -1,13 +1,21 @@ export function debounce(fn: (...args: A[]) => R, delay: number) { let timeout: NodeJS.Timeout | null = null - return function debounceImpl(...args: A[]): void { + const cancel = () => { if (timeout) { clearTimeout(timeout) } + } + + function debounceImpl(...args: A[]): void { + cancel() timeout = setTimeout(() => { fn(...args) }, delay) } + + debounceImpl.cancel = cancel + + return debounceImpl }