diff --git a/lib/schedule.explorer.ts b/lib/schedule.explorer.ts index 7c17d1ca..f6833aff 100644 --- a/lib/schedule.explorer.ts +++ b/lib/schedule.explorer.ts @@ -26,19 +26,20 @@ export class ScheduleExplorer implements OnModuleInit { ...this.discoveryService.getControllers(), ...this.discoveryService.getProviders(), ]; - instanceWrappers - .filter((wrapper) => wrapper.isDependencyTreeStatic()) - .forEach((wrapper: InstanceWrapper) => { - const { instance } = wrapper; - if (!instance || !Object.getPrototypeOf(instance)) { - return; - } - this.metadataScanner.scanFromPrototype( - instance, - Object.getPrototypeOf(instance), - (key: string) => this.lookupSchedulers(instance, key), - ); - }); + instanceWrappers.forEach((wrapper: InstanceWrapper) => { + const { instance } = wrapper; + if (!instance || !Object.getPrototypeOf(instance)) { + return; + } + this.metadataScanner.scanFromPrototype( + instance, + Object.getPrototypeOf(instance), + (key: string) => + wrapper.isDependencyTreeStatic() + ? this.lookupSchedulers(instance, key) + : this.warnForNonStaticProviders(wrapper, instance, key), + ); + }); } lookupSchedulers(instance: Record, key: string) { @@ -87,6 +88,36 @@ export class ScheduleExplorer implements OnModuleInit { } } + warnForNonStaticProviders( + wrapper: InstanceWrapper, + instance: Record, + key: string, + ) { + const methodRef = instance[key]; + const metadata = this.metadataAccessor.getSchedulerType(methodRef); + + switch (metadata) { + case SchedulerType.CRON: { + this.logger.warn( + `Cannot register cron job "${wrapper.name}@${key}" because it is defined in a non static provider.`, + ); + break; + } + case SchedulerType.TIMEOUT: { + this.logger.warn( + `Cannot register timeout "${wrapper.name}@${key}" because it is defined in a non static provider.`, + ); + break; + } + case SchedulerType.INTERVAL: { + this.logger.warn( + `Cannot register interval "${wrapper.name}@${key}" because it is defined in a non static provider.`, + ); + break; + } + } + } + private wrapFunctionInTryCatchBlocks(methodRef: Function, instance: object) { return async (...args: unknown[]) => { try { diff --git a/tests/e2e/cron-jobs.spec.ts b/tests/e2e/cron-jobs.spec.ts index 080b8996..c936df56 100644 --- a/tests/e2e/cron-jobs.spec.ts +++ b/tests/e2e/cron-jobs.spec.ts @@ -1,9 +1,10 @@ -import { INestApplication } from '@nestjs/common'; +import { INestApplication, Logger } from '@nestjs/common'; import { Test } from '@nestjs/testing'; import sinon from 'sinon'; import { SchedulerRegistry } from '../../lib/scheduler.registry'; import { AppModule } from '../src/app.module'; import { CronService } from '../src/cron.service'; +import { RequestScopedCronService } from '../src/request-scoped-cron.service'; import { nullPrototypeObjectProvider } from '../src/null-prototype-object.provider'; const deleteAllRegisteredJobsExceptOne = ( @@ -186,6 +187,57 @@ describe('Cron', () => { expect(service.doesExists('dynamic')).toEqual(false); }); + it(`should not log a warning when the provider is not request scoped`, async () => { + const logger = { + log: jest.fn(), + error: jest.fn(), + warn: jest.fn(), + }; + Logger.overrideLogger(logger); + jest.spyOn(logger, 'warn'); + + await app.init(); + + expect(logger.warn).not.toHaveBeenCalled(); + }); + + afterEach(async () => { + await app.close(); + }); +}); + +describe('Cron - Request Scoped Provider', () => { + let app: INestApplication; + let clock: sinon.SinonFakeTimers; + + beforeEach(async () => { + const module = await Test.createTestingModule({ + imports: [AppModule.registerRequestScopedCron()], + }).compile(); + + app = module.createNestApplication(); + clock = sinon.useFakeTimers({ now: 1577836800000 }); // 2020-01-01T00:00:00.000Z + }); + + it(`should log a warning when trying to register a cron in a request scoped provider`, async () => { + const logger = { + log: jest.fn(), + error: jest.fn(), + warn: jest.fn(), + }; + Logger.overrideLogger(logger); + jest.spyOn(logger, 'warn'); + + await app.init(); + const registry = app.get(SchedulerRegistry); + + expect(registry.getCronJobs()).toEqual(new Map()); + expect(logger.warn).toHaveBeenCalledWith( + 'Cannot register cron job "RequestScopedCronService@handleCron" because it is defined in a non static provider.', + 'Scheduler', + ); + }); + afterEach(async () => { await app.close(); }); diff --git a/tests/e2e/interval.spec.ts b/tests/e2e/interval.spec.ts index 31532482..dece423e 100644 --- a/tests/e2e/interval.spec.ts +++ b/tests/e2e/interval.spec.ts @@ -1,9 +1,10 @@ -import { INestApplication } from '@nestjs/common'; +import { INestApplication, Logger } from '@nestjs/common'; import { Test } from '@nestjs/testing'; import { SchedulerRegistry } from '../../lib/scheduler.registry'; import { AppModule } from '../src/app.module'; import { IntervalService } from '../src/interval.service'; import { nullPrototypeObjectProvider } from '../src/null-prototype-object.provider'; +import { RequestScopedIntervalService } from '../src/request-scoped-interval.service'; describe('Interval', () => { let app: INestApplication; @@ -112,6 +113,56 @@ describe('Interval', () => { expect(service.doesExists('dynamic')).toEqual(false); }); + it(`should not log a warning when the provider is not request scoped`, async () => { + const logger = { + log: jest.fn(), + error: jest.fn(), + warn: jest.fn(), + }; + Logger.overrideLogger(logger); + jest.spyOn(logger, 'warn'); + + await app.init(); + + expect(logger.warn).not.toHaveBeenCalledWith(); + }); + + afterEach(async () => { + await app.close(); + }); +}); + +describe('Interval - Request Scoped', () => { + let app: INestApplication; + + beforeEach(async () => { + const module = await Test.createTestingModule({ + imports: [AppModule.registerRequestScopedInterval()], + }).compile(); + + app = module.createNestApplication(); + jest.useFakeTimers(); + }); + + it(`should log a warning when trying to register an interval in a request scoped provider`, async () => { + const logger = { + log: jest.fn(), + error: jest.fn(), + warn: jest.fn(), + }; + Logger.overrideLogger(logger); + jest.spyOn(logger, 'warn'); + + await app.init(); + const registry = app.get(SchedulerRegistry); + + expect(registry.getIntervals()).toEqual([]); + expect(logger.warn).toHaveBeenCalledWith( + 'Cannot register interval "RequestScopedIntervalService@handleInterval" because it is defined in a non static provider.', + 'Scheduler', + ); + }); + afterEach(async () => { await app.close(); }); diff --git a/tests/e2e/timeout.spec.ts b/tests/e2e/timeout.spec.ts index 9e10d2f9..e1463624 100644 --- a/tests/e2e/timeout.spec.ts +++ b/tests/e2e/timeout.spec.ts @@ -1,8 +1,9 @@ -import { INestApplication } from '@nestjs/common'; +import { INestApplication, Logger } from '@nestjs/common'; import { Test } from '@nestjs/testing'; import { SchedulerRegistry } from '../../lib/scheduler.registry'; import { AppModule } from '../src/app.module'; import { nullPrototypeObjectProvider } from '../src/null-prototype-object.provider'; +import { RequestScopedTimeoutService } from '../src/request-scoped-timeout.service'; import { TimeoutService } from '../src/timeout.service'; describe('Timeout', () => { @@ -112,6 +113,56 @@ describe('Timeout', () => { expect(service.doesExists('dynamic')).toEqual(false); }); + it(`should not log a warning when the provider is not request scoped`, async () => { + const logger = { + log: jest.fn(), + error: jest.fn(), + warn: jest.fn(), + }; + Logger.overrideLogger(logger); + jest.spyOn(logger, 'warn'); + + await app.init(); + + expect(logger.warn).not.toHaveBeenCalledWith(); + }); + + afterEach(async () => { + await app.close(); + }); +}); + +describe('Timeout - Request Scoped', () => { + let app: INestApplication; + + beforeEach(async () => { + const module = await Test.createTestingModule({ + imports: [AppModule.registerRequestScopedTimeout()], + }).compile(); + + app = module.createNestApplication(); + jest.useFakeTimers(); + }); + + it(`should log a warning when trying to register an timeout in a request scoped provider`, async () => { + const logger = { + log: jest.fn(), + error: jest.fn(), + warn: jest.fn(), + }; + Logger.overrideLogger(logger); + jest.spyOn(logger, 'warn'); + + await app.init(); + const registry = app.get(SchedulerRegistry); + + expect(registry.getTimeouts()).toEqual([]); + expect(logger.warn).toHaveBeenCalledWith( + 'Cannot register timeout "RequestScopedTimeoutService@handleTimeout" because it is defined in a non static provider.', + 'Scheduler', + ); + }); + afterEach(async () => { await app.close(); }); diff --git a/tests/src/app.module.ts b/tests/src/app.module.ts index fe8e2095..7ea26b32 100644 --- a/tests/src/app.module.ts +++ b/tests/src/app.module.ts @@ -2,6 +2,9 @@ import { DynamicModule, Module } from '@nestjs/common'; import { ScheduleModule } from '../../lib/schedule.module'; import { CronService } from './cron.service'; import { IntervalService } from './interval.service'; +import { RequestScopedCronService } from './request-scoped-cron.service'; +import { RequestScopedIntervalService } from './request-scoped-interval.service'; +import { RequestScopedTimeoutService } from './request-scoped-timeout.service'; import { TimeoutService } from './timeout.service'; @Module({}) @@ -14,6 +17,14 @@ export class AppModule { }; } + static registerRequestScopedTimeout(): DynamicModule { + return { + module: AppModule, + imports: [ScheduleModule.forRoot()], + providers: [RequestScopedTimeoutService], + }; + } + static registerInterval(): DynamicModule { return { module: AppModule, @@ -22,6 +33,14 @@ export class AppModule { }; } + static registerRequestScopedInterval(): DynamicModule { + return { + module: AppModule, + imports: [ScheduleModule.forRoot()], + providers: [RequestScopedIntervalService], + }; + } + static registerCron(): DynamicModule { return { module: AppModule, @@ -29,4 +48,12 @@ export class AppModule { providers: [CronService], }; } + + static registerRequestScopedCron(): DynamicModule { + return { + module: AppModule, + imports: [ScheduleModule.forRoot()], + providers: [RequestScopedCronService], + }; + } } diff --git a/tests/src/request-scoped-cron.service.ts b/tests/src/request-scoped-cron.service.ts new file mode 100644 index 00000000..78d0a0ed --- /dev/null +++ b/tests/src/request-scoped-cron.service.ts @@ -0,0 +1,9 @@ +import { Injectable, Scope } from '@nestjs/common'; +import { Cron } from '../../lib/decorators'; +import { CronExpression } from '../../lib/enums'; + +@Injectable({ scope: Scope.REQUEST }) +export class RequestScopedCronService { + @Cron(CronExpression.EVERY_MINUTE) + handleCron() {} +} diff --git a/tests/src/request-scoped-interval.service.ts b/tests/src/request-scoped-interval.service.ts new file mode 100644 index 00000000..acc10c36 --- /dev/null +++ b/tests/src/request-scoped-interval.service.ts @@ -0,0 +1,8 @@ +import { Injectable, Scope } from '@nestjs/common'; +import { Interval } from '../../lib/decorators'; + +@Injectable({ scope: Scope.REQUEST }) +export class RequestScopedIntervalService { + @Interval('test', 2500) + handleInterval() {} +} diff --git a/tests/src/request-scoped-timeout.service.ts b/tests/src/request-scoped-timeout.service.ts new file mode 100644 index 00000000..34c13ca9 --- /dev/null +++ b/tests/src/request-scoped-timeout.service.ts @@ -0,0 +1,8 @@ +import { Injectable, Scope } from '@nestjs/common'; +import { Timeout } from '../../lib/decorators'; + +@Injectable({ scope: Scope.REQUEST }) +export class RequestScopedTimeoutService { + @Timeout('test', 2500) + handleTimeout() {} +}