Skip to content

Commit 769ca03

Browse files
AlessioGrAlessioGr
authored
fix: ensure job autoruns are not triggered if jobs collection not enabled (#12808)
Fixes #12776 - Adds a new `jobs.config.enabled` property to the sanitized config, which can be used to easily check if the jobs system is enabled (i.e., if the payload-jobs collection was added during sanitization). This is then checked before Payload sets up job autoruns. - Fixes some type issues that occurred due to still deep-requiring the jobs config - we forgot to omit it from the `DeepRequired(Config)` call. The deep-required jobs config was then incorrectly merged with the sanitized jobs config, resulting in a SanitizedConfig where all jobs config properties were marked as required, even though they may be undefined.
1 parent 4e2e4d2 commit 769ca03

File tree

7 files changed

+80
-75
lines changed

7 files changed

+80
-75
lines changed

‎packages/payload/src/config/sanitize.ts

Lines changed: 34 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { AcceptedLanguages } from '@payloadcms/translations'
33
import { en } from '@payloadcms/translations/languages/en'
44
import { deepMergeSimple } from '@payloadcms/translations/utilities'
55

6+
import type { SanitizedJobsConfig } from '../queues/config/types/index.js'
67
import type {
78
Config,
89
LocalizationConfigWithLabels,
@@ -281,12 +282,8 @@ export const sanitizeConfig = async (incomingConfig: Config): Promise<SanitizedC
281282
}
282283
}
283284

284-
if (schedulePublishCollections.length > 0 || schedulePublishGlobals.length > 0) {
285-
if (!Array.isArray(configWithDefaults.jobs?.tasks)) {
286-
configWithDefaults.jobs!.tasks = []
287-
}
288-
289-
configWithDefaults.jobs!.tasks.push(
285+
if (schedulePublishCollections.length || schedulePublishGlobals.length) {
286+
;((config.jobs ??= {} as SanitizedJobsConfig).tasks ??= []).push(
290287
getSchedulePublishTask({
291288
adminUserSlug: config.admin!.user,
292289
collections: schedulePublishCollections,
@@ -295,44 +292,44 @@ export const sanitizeConfig = async (incomingConfig: Config): Promise<SanitizedC
295292
)
296293
}
297294

298-
// Need to add default jobs collection before locked documents collections
299-
if (
295+
;(config.jobs ??= {} as SanitizedJobsConfig).enabled = Boolean(
300296
(Array.isArray(configWithDefaults.jobs?.tasks) && configWithDefaults.jobs?.tasks?.length) ||
301-
(Array.isArray(configWithDefaults.jobs?.workflows) &&
302-
configWithDefaults.jobs?.workflows?.length)
303-
) {
297+
(Array.isArray(configWithDefaults.jobs?.workflows) &&
298+
configWithDefaults.jobs?.workflows?.length),
299+
)
300+
301+
// Need to add default jobs collection before locked documents collections
302+
if (config.jobs.enabled) {
304303
let defaultJobsCollection = getDefaultJobsCollection(config as unknown as Config)
305304

306-
if (defaultJobsCollection) {
307-
if (typeof configWithDefaults.jobs.jobsCollectionOverrides === 'function') {
308-
defaultJobsCollection = configWithDefaults.jobs.jobsCollectionOverrides({
309-
defaultJobsCollection,
310-
})
311-
312-
const hooks = defaultJobsCollection?.hooks
313-
// @todo - delete this check in 4.0
314-
if (hooks && config?.jobs?.runHooks !== true) {
315-
for (const hook of Object.keys(hooks)) {
316-
const defaultAmount = hook === 'afterRead' || hook === 'beforeChange' ? 1 : 0
317-
if (hooks[hook as keyof typeof hooks]!.length > defaultAmount) {
318-
// eslint-disable-next-line no-console
319-
console.warn(
320-
`The jobsCollectionOverrides function is returning a collection with an additional ${hook} hook defined. These hooks will not run unless the jobs.runHooks option is set to true. Setting this option to true will negatively impact performance.`,
321-
)
322-
break
323-
}
305+
if (typeof config.jobs.jobsCollectionOverrides === 'function') {
306+
defaultJobsCollection = config.jobs.jobsCollectionOverrides({
307+
defaultJobsCollection,
308+
})
309+
310+
const hooks = defaultJobsCollection?.hooks
311+
// @todo - delete this check in 4.0
312+
if (hooks && config?.jobs?.runHooks !== true) {
313+
for (const [hookKey, hook] of Object.entries(hooks)) {
314+
const defaultAmount = hookKey === 'afterRead' || hookKey === 'beforeChange' ? 1 : 0
315+
if (hook.length > defaultAmount) {
316+
// eslint-disable-next-line no-console
317+
console.warn(
318+
`The jobsCollectionOverrides function is returning a collection with an additional ${hookKey} hook defined. These hooks will not run unless the jobs.runHooks option is set to true. Setting this option to true will negatively impact performance.`,
319+
)
320+
break
324321
}
325322
}
326323
}
327-
const sanitizedJobsCollection = await sanitizeCollection(
328-
config as unknown as Config,
329-
defaultJobsCollection,
330-
richTextSanitizationPromises,
331-
validRelationships,
332-
)
333-
334-
configWithDefaults.collections!.push(sanitizedJobsCollection)
335324
}
325+
const sanitizedJobsCollection = await sanitizeCollection(
326+
config as unknown as Config,
327+
defaultJobsCollection,
328+
richTextSanitizationPromises,
329+
validRelationships,
330+
)
331+
332+
configWithDefaults.collections!.push(sanitizedJobsCollection)
336333
}
337334

338335
configWithDefaults.collections!.push(

‎packages/payload/src/config/types.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import type {
5151
TypedUser,
5252
} from '../index.js'
5353
import type { QueryPreset, QueryPresetConstraints } from '../query-presets/types.js'
54+
import type { SanitizedJobsConfig } from '../queues/config/types/index.js'
5455
import type { PayloadRequest, Where } from '../types/index.js'
5556
import type { PayloadLogger } from '../utilities/logger.js'
5657

@@ -1250,7 +1251,7 @@ export type SanitizedConfig = {
12501251
endpoints: Endpoint[]
12511252
globals: SanitizedGlobalConfig[]
12521253
i18n: Required<I18nOptions>
1253-
jobs: JobsConfig // Redefine here, as the DeepRequired<Config> can break its type
1254+
jobs: SanitizedJobsConfig
12541255
localization: false | SanitizedLocalizationConfig
12551256
paths: {
12561257
config: string
@@ -1275,6 +1276,7 @@ export type SanitizedConfig = {
12751276
| 'endpoint'
12761277
| 'globals'
12771278
| 'i18n'
1279+
| 'jobs'
12781280
| 'localization'
12791281
| 'upload'
12801282
>

‎packages/payload/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -772,7 +772,7 @@ export class BasePayload {
772772
throw error
773773
}
774774

775-
if (this.config.jobs.autoRun && !isNextBuild()) {
775+
if (this.config.jobs.enabled && this.config.jobs.autoRun && !isNextBuild()) {
776776
const DEFAULT_CRON = '* * * * *'
777777
const DEFAULT_LIMIT = 10
778778

‎packages/payload/src/queues/config/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { getJobTaskStatus } from '../utilities/getJobTaskStatus.js'
88

99
export const jobsCollectionSlug = 'payload-jobs'
1010

11-
export const getDefaultJobsCollection: (config: Config) => CollectionConfig | null = (config) => {
11+
export const getDefaultJobsCollection: (config: Config) => CollectionConfig = (config) => {
1212
const workflowSlugs: Set<string> = new Set()
1313
const taskSlugs: Set<string> = new Set(['inline'])
1414

‎packages/payload/src/queues/config/types/index.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,13 @@ export type RunJobAccessArgs = {
4242

4343
export type RunJobAccess = (args: RunJobAccessArgs) => boolean | Promise<boolean>
4444

45+
export type SanitizedJobsConfig = {
46+
/**
47+
* If set to `true`, the job system is enabled and a payload-jobs collection exists.
48+
* This property is automatically set during sanitization.
49+
*/
50+
enabled?: boolean
51+
} & JobsConfig
4552
export type JobsConfig = {
4653
/**
4754
* Specify access control to determine who can interact with jobs.

‎packages/payload/src/queues/operations/runJobs/index.ts

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,19 @@ export const runJobs = async (args: RunJobsArgs): Promise<RunJobsResult> => {
8080
processingOrder,
8181
queue = 'default',
8282
req,
83+
req: {
84+
payload,
85+
payload: {
86+
config: { jobs: jobsConfig },
87+
},
88+
},
8389
sequential,
8490
where: whereFromProps,
8591
} = args
8692

8793
if (!overrideAccess) {
88-
const hasAccess = await req.payload.config.jobs.access.run({ req })
94+
const accessFn = jobsConfig?.access?.run ?? (() => true)
95+
const hasAccess = await accessFn({ req })
8996
if (!hasAccess) {
9097
throw new Forbidden(req.t)
9198
}
@@ -150,17 +157,17 @@ export const runJobs = async (args: RunJobsArgs): Promise<RunJobsResult> => {
150157
data: {
151158
processing: true,
152159
},
153-
depth: req.payload.config.jobs.depth,
160+
depth: jobsConfig.depth,
154161
disableTransaction: true,
155162
req,
156163
returning: true,
157164
}))!,
158165
]
159166
} else {
160167
let defaultProcessingOrder: Sort =
161-
req.payload.collections[jobsCollectionSlug]?.config.defaultSort ?? 'createdAt'
168+
payload.collections[jobsCollectionSlug]?.config.defaultSort ?? 'createdAt'
162169

163-
const processingOrderConfig = req.payload.config.jobs?.processingOrder
170+
const processingOrderConfig = jobsConfig?.processingOrder
164171
if (typeof processingOrderConfig === 'function') {
165172
defaultProcessingOrder = await processingOrderConfig(args)
166173
} else if (typeof processingOrderConfig === 'object' && !Array.isArray(processingOrderConfig)) {
@@ -181,7 +188,7 @@ export const runJobs = async (args: RunJobsArgs): Promise<RunJobsResult> => {
181188
data: {
182189
processing: true,
183190
},
184-
depth: req.payload.config.jobs.depth,
191+
depth: jobsConfig.depth,
185192
disableTransaction: true,
186193
limit,
187194
req,
@@ -219,13 +226,13 @@ export const runJobs = async (args: RunJobsArgs): Promise<RunJobsResult> => {
219226
}
220227

221228
if (jobsQuery?.docs?.length) {
222-
req.payload.logger.info({
229+
payload.logger.info({
223230
msg: `Running ${jobsQuery.docs.length} jobs.`,
224231
new: newJobs?.length,
225232
retrying: existingJobs?.length,
226233
})
227234
}
228-
const jobsToDelete: (number | string)[] | undefined = req.payload.config.jobs.deleteJobOnComplete
235+
const jobsToDelete: (number | string)[] | undefined = jobsConfig.deleteJobOnComplete
229236
? []
230237
: undefined
231238

@@ -235,16 +242,17 @@ export const runJobs = async (args: RunJobsArgs): Promise<RunJobsResult> => {
235242
}
236243
const jobReq = isolateObjectProperty(req, 'transactionID')
237244

238-
const workflowConfig: WorkflowConfig<WorkflowTypes> = job.workflowSlug
239-
? req.payload.config.jobs.workflows.find(({ slug }) => slug === job.workflowSlug)!
240-
: {
241-
slug: 'singleTask',
242-
handler: async ({ job, tasks }) => {
243-
await tasks[job.taskSlug as string]!('1', {
244-
input: job.input,
245-
})
246-
},
247-
}
245+
const workflowConfig: WorkflowConfig<WorkflowTypes> =
246+
job.workflowSlug && jobsConfig.workflows?.length
247+
? jobsConfig.workflows.find(({ slug }) => slug === job.workflowSlug)!
248+
: {
249+
slug: 'singleTask',
250+
handler: async ({ job, tasks }) => {
251+
await tasks[job.taskSlug as string]!('1', {
252+
input: job.input,
253+
})
254+
},
255+
}
248256

249257
if (!workflowConfig) {
250258
return null // Skip jobs with no workflow configuration
@@ -268,7 +276,7 @@ export const runJobs = async (args: RunJobsArgs): Promise<RunJobsResult> => {
268276
if (!workflowHandler) {
269277
const jobLabel = job.workflowSlug || `Task: ${job.taskSlug}`
270278
const errorMessage = `Can't find runner while importing with the path ${workflowConfig.handler} in job type ${jobLabel}.`
271-
req.payload.logger.error(errorMessage)
279+
payload.logger.error(errorMessage)
272280

273281
await updateJob({
274282
error: {
@@ -331,21 +339,21 @@ export const runJobs = async (args: RunJobsArgs): Promise<RunJobsResult> => {
331339

332340
if (jobsToDelete && jobsToDelete.length > 0) {
333341
try {
334-
if (req.payload.config.jobs.runHooks) {
335-
await req.payload.delete({
342+
if (jobsConfig.runHooks) {
343+
await payload.delete({
336344
collection: jobsCollectionSlug,
337345
depth: 0, // can be 0 since we're not returning anything
338346
disableTransaction: true,
339347
where: { id: { in: jobsToDelete } },
340348
})
341349
} else {
342-
await req.payload.db.deleteMany({
350+
await payload.db.deleteMany({
343351
collection: jobsCollectionSlug,
344352
where: { id: { in: jobsToDelete } },
345353
})
346354
}
347355
} catch (err) {
348-
req.payload.logger.error({
356+
payload.logger.error({
349357
err,
350358
msg: `failed to delete jobs ${jobsToDelete.join(', ')} on complete`,
351359
})

‎packages/payload/src/queues/restEndpointRun.ts

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,7 @@ import type { Endpoint, SanitizedConfig } from '../config/types.js'
33
import { runJobs, type RunJobsArgs } from './operations/runJobs/index.js'
44

55
const configHasJobs = (config: SanitizedConfig): boolean => {
6-
if (!config.jobs) {
7-
return false
8-
}
9-
10-
if (config.jobs.tasks?.length > 0) {
11-
return true
12-
}
13-
if (config.jobs.workflows?.length > 0) {
14-
return true
15-
}
16-
17-
return false
6+
return Boolean(config.jobs?.tasks?.length || config.jobs?.workflows?.length)
187
}
198

209
export const runJobsEndpoint: Endpoint = {
@@ -28,7 +17,9 @@ export const runJobsEndpoint: Endpoint = {
2817
)
2918
}
3019

31-
const hasAccess = await req.payload.config.jobs.access.run({ req })
20+
const accessFn = req.payload.config.jobs?.access?.run ?? (() => true)
21+
22+
const hasAccess = await accessFn({ req })
3223

3324
if (!hasAccess) {
3425
return Response.json(

0 commit comments

Comments
 (0)