diff --git a/packages/jobs/src/core/JobManager.ts b/packages/jobs/src/core/JobManager.ts index 04f824a34438..3a14f534689d 100644 --- a/packages/jobs/src/core/JobManager.ts +++ b/packages/jobs/src/core/JobManager.ts @@ -53,12 +53,13 @@ export class JobManager< logger: this.logger, }) - return >( - job: T, - jobArgs?: Parameters, - jobOptions?: ScheduleJobOptions, + return >( + job: TJob, + ...argsAndOptions: Parameters extends [] + ? [undefined?, ScheduleJobOptions?] + : [Parameters, ScheduleJobOptions?] ) => { - return scheduler.schedule({ job, jobArgs, jobOptions }) + return scheduler.schedule(job, ...argsAndOptions) } } diff --git a/packages/jobs/src/core/Scheduler.ts b/packages/jobs/src/core/Scheduler.ts index e6bcdeda6f5c..66231603024d 100644 --- a/packages/jobs/src/core/Scheduler.ts +++ b/packages/jobs/src/core/Scheduler.ts @@ -50,9 +50,11 @@ export class Scheduler { buildPayload>( job: TJob, - args?: Parameters, - options?: ScheduleJobOptions, + ...argsAndOptions: Parameters extends [] + ? [undefined?, ScheduleJobOptions?] + : [Parameters, ScheduleJobOptions?] ): SchedulePayload { + const [args, options] = argsAndOptions const queue = job.queue const priority = job.priority ?? DEFAULT_PRIORITY const wait = options?.wait ?? DEFAULT_WAIT @@ -72,16 +74,13 @@ export class Scheduler { } } - async schedule>({ - job, - jobArgs, - jobOptions, - }: { - job: TJob - jobArgs?: Parameters - jobOptions?: ScheduleJobOptions - }) { - const payload = this.buildPayload(job, jobArgs, jobOptions) + async schedule>( + job: TJob, + ...argsAndOptions: Parameters extends [] + ? [undefined?, ScheduleJobOptions?] + : [Parameters, ScheduleJobOptions?] + ) { + const payload = this.buildPayload(job, ...argsAndOptions) this.logger.info(payload, `[RedwoodJob] Scheduling ${job.name}`) diff --git a/packages/jobs/src/core/__tests__/JobManager.test.ts b/packages/jobs/src/core/__tests__/JobManager.test.ts index 6ab41e28e042..c28e67d534e3 100644 --- a/packages/jobs/src/core/__tests__/JobManager.test.ts +++ b/packages/jobs/src/core/__tests__/JobManager.test.ts @@ -125,11 +125,111 @@ describe('createScheduler()', () => { scheduler(mockJob, mockArgs, mockOptions) - expect(Scheduler.prototype.schedule).toHaveBeenCalledWith({ - job: mockJob, - jobArgs: mockArgs, - jobOptions: mockOptions, + expect(Scheduler.prototype.schedule).toHaveBeenCalledWith( + mockJob, + mockArgs, + mockOptions, + ) + }) + + it('returns a function that takes an array of arguments to pass to perform()', () => { + const manager = new JobManager({ + adapters: { + mock: mockAdapter, + }, + queues: ['default'] as const, + logger: mockLogger, + workers: [], + }) + + interface MockJobArgs { + foo: string + bar: number + } + + const mockJob = manager.createJob({ + queue: 'default', + perform: ({ foo, bar }: MockJobArgs) => { + return void (foo + bar) + }, + }) + + const scheduler = manager.createScheduler({ adapter: 'mock' }) + + // Should be correct. No red squiggly lines + scheduler(mockJob, [{ foo: 'foo', bar: 645 }], { wait: 30 }) + + // Uncomment the line below and you should see an error because passing + // `undefined` should not be allowed when arguments are required + // scheduler(mockJob, undefined, { wait: 30 }) + + // Uncomment the line below and you should see an error because not passing + // anything should not be allowed when arguments are required + // scheduler(mockJob) + }) + + it("returns a function that doesn't need arguments to pass to perform()", () => { + const manager = new JobManager({ + adapters: { + mock: mockAdapter, + }, + queues: ['default'] as const, + logger: mockLogger, + workers: [], + }) + + const mockJob = manager.createJob({ + queue: 'default', + perform: () => { + return void 'no args' + }, + }) + + const scheduler = manager.createScheduler({ adapter: 'mock' }) + + // Should be correct + scheduler(mockJob, undefined, { wait: 30 }) + // Should be correct + scheduler(mockJob, undefined) + // Should be correct + scheduler(mockJob) + + // Uncomment the line below and you'll see an error right now. When the + // job's perform() method doesn't take any arguments you're not allowed to + // pass it an empty array. We could possibly allow this in the future if + // we want to, but for now it's an error. + // scheduler(mockJob, [], { wait: 30 }) + }) + + it('returns a function with only optional arguments to pass to perform()', () => { + const manager = new JobManager({ + adapters: { + mock: mockAdapter, + }, + queues: ['default'] as const, + logger: mockLogger, + workers: [], + }) + + const mockJob = manager.createJob({ + queue: 'default', + perform: (first?: string, second?: string) => { + return void (first || '' + second) + }, }) + + const scheduler = manager.createScheduler({ adapter: 'mock' }) + + // Should be correct + scheduler(mockJob, ['1st', '2nd']) + // Should be correct + scheduler(mockJob, []) + + // Uncomment any of the lines below and you'll see an error. Ideally I think + // this should be allowed, because all arguments are optional. But on this + // first iteration I couldn't figure out how to make that work. + // scheduler(mockJob, undefined) + // scheduler(mockJob) }) }) diff --git a/packages/jobs/src/core/__tests__/Scheduler.test.ts b/packages/jobs/src/core/__tests__/Scheduler.test.ts index 03b01dc92ae7..223a267235cc 100644 --- a/packages/jobs/src/core/__tests__/Scheduler.test.ts +++ b/packages/jobs/src/core/__tests__/Scheduler.test.ts @@ -208,7 +208,7 @@ describe('schedule()', () => { wait: 10, } - await scheduler.schedule({ job, jobArgs: args, jobOptions: options }) + await scheduler.schedule(job, args, options) expect(mockAdapter.schedule).toHaveBeenCalledWith( expect.objectContaining({ @@ -240,8 +240,8 @@ describe('schedule()', () => { wait: 10, } - await expect( - scheduler.schedule({ job, jobArgs: args, jobOptions: options }), - ).rejects.toThrow(errors.SchedulingError) + await expect(scheduler.schedule(job, args, options)).rejects.toThrow( + errors.SchedulingError, + ) }) })