This is an automated email from the ASF dual-hosted git repository.
jdaugherty pushed a commit to branch 8.0.x
in repository https://gitbox.apache.org/repos/asf/grails-core.git
The following commit(s) were added to refs/heads/8.0.x by this push:
new fa68e5eb7f Prevent null intervals
fa68e5eb7f is described below
commit fa68e5eb7fb58a7521948d12710507645b9de686
Author: James Daugherty <[email protected]>
AuthorDate: Sat Aug 22 23:41:05 2026 -0400
Prevent null intervals
---
.../quartzDynamicScheduling.adoc | 10 +
.../groovy/grails/plugins/quartz/QuartzJob.groovy | 60 +++-
.../grails/plugins/quartz/QuartzJobSpec.groovy | 325 +++++++++++++++++++++
.../grails-app/jobs/quartzapp/DisabledJob.groovy | 31 ++
.../groovy/quartzapp/QuartzSchedulingSpec.groovy | 48 +++
5 files changed, 472 insertions(+), 2 deletions(-)
diff --git
a/grails-doc/src/en/guide/backgroundJobs/backgroundJobsAdvanced/quartzDynamicScheduling.adoc
b/grails-doc/src/en/guide/backgroundJobs/backgroundJobsAdvanced/quartzDynamicScheduling.adoc
index 8fe6feea39..5fe6ec07a8 100644
---
a/grails-doc/src/en/guide/backgroundJobs/backgroundJobsAdvanced/quartzDynamicScheduling.adoc
+++
b/grails-doc/src/en/guide/backgroundJobs/backgroundJobsAdvanced/quartzDynamicScheduling.adoc
@@ -79,3 +79,13 @@ class MyController {
When a trigger is passed directly to `schedule`, its job key is rewritten to
point at the job class, so the trigger must be mutable — that is, an instance
of `org.quartz.spi.MutableTrigger`, which every trigger implementation shipped
with Quartz is. A trigger that is neither mutable nor already keyed to the job
is rejected.
The default trigger group used by `unschedule` is `GRAILS_TRIGGERS`.
+
+These methods only work on a job that the plugin has registered with the
scheduler, which it does for every enabled job artefact while the application
starts. Calling them on a job that is turned off with `jobEnabled = false`, or
on a class the application does not manage as a job artefact, throws an
`IllegalStateException` reporting that the job is not registered with a
scheduler.
+
+A null argument is rejected with an `IllegalArgumentException` that names the
argument which is missing. Since the runtime type of `null` carries no
information, a null passed by a caller that is not statically compiled always
resolves to `schedule(Trigger)`, whichever method the caller meant to reach:
+
+[source,groovy]
+----
+// reports a null trigger, even though the interval read from the
configuration is what is null
+MyJob.schedule(grailsApplication.config.getProperty('my.job.repeatInterval',
Long))
+----
diff --git
a/grails-quartz/src/main/groovy/grails/plugins/quartz/QuartzJob.groovy
b/grails-quartz/src/main/groovy/grails/plugins/quartz/QuartzJob.groovy
index d77e554fc2..b6b468a83b 100644
--- a/grails-quartz/src/main/groovy/grails/plugins/quartz/QuartzJob.groovy
+++ b/grails-quartz/src/main/groovy/grails/plugins/quartz/QuartzJob.groovy
@@ -37,30 +37,43 @@ trait QuartzJob implements GrailsApplicationAware {
GrailsApplication grailsApplication
static triggerNow(Map params = null) {
+ assertScheduled this.getName()
internalScheduler.triggerJob(new JobKey(this.getName(),
internalJobArtefact.group), params ? new JobDataMap(params) : null)
}
@CompileDynamic
static schedule(Long repeatInterval, Integer repeatCount =
SimpleTrigger.REPEAT_INDEFINITELY, Map params = null) {
+ assertScheduled this.getName()
+ Assert.notNull repeatInterval,
missingScheduleArgumentMessage(this.getName(), 'repeat interval')
+ Assert.notNull repeatCount,
missingScheduleArgumentMessage(this.getName(), 'repeat count')
internalScheduleTrigger(TriggerUtils.buildSimpleTrigger(this.getName(),
internalJobArtefact.group, repeatInterval, repeatCount), params)
}
@CompileDynamic
static schedule(Date scheduleDate, Map params = null) {
+ assertScheduled this.getName()
+ Assert.notNull scheduleDate,
missingScheduleArgumentMessage(this.getName(), 'schedule date')
internalScheduleTrigger(TriggerUtils.buildDateTrigger(this.getName(),
internalJobArtefact.group, scheduleDate), params)
}
@CompileDynamic
static schedule(String cronExpression, Map params = null) {
+ assertScheduled this.getName()
+ Assert.notNull cronExpression,
missingScheduleArgumentMessage(this.getName(), 'cron expression')
internalScheduleTrigger(TriggerUtils.buildCronTrigger(this.getName(),
internalJobArtefact.group, cronExpression), params)
}
static schedule(Trigger trigger, Map params = null) {
- def jobKey = new JobKey(this.getName(), internalJobArtefact.group)
+ assertScheduled this.getName()
+ Assert.notNull trigger, missingScheduleArgumentMessage(this.getName(),
'trigger')
+
+ JobKey jobKey = new JobKey(this.getName(), internalJobArtefact.group)
Assert.isTrue trigger.jobKey == jobKey || (trigger instanceof
MutableTrigger),
'The trigger job key is not equal to the job key or the
trigger is immutable'
- ((MutableTrigger)trigger).jobKey = jobKey
+ if (trigger instanceof MutableTrigger) {
+ ((MutableTrigger) trigger).jobKey = jobKey
+ }
if (params) {
trigger.jobDataMap.putAll(params)
@@ -69,15 +82,20 @@ trait QuartzJob implements GrailsApplicationAware {
}
static removeJob() {
+ assertScheduled this.getName()
internalScheduler.deleteJob(new JobKey(this.getName(),
internalJobArtefact.group))
}
static reschedule(Trigger trigger, Map params = null) {
+ assertScheduled this.getName()
+ Assert.notNull trigger, missingArgumentMessage(this.getName(),
'trigger')
if (params) trigger.jobDataMap.putAll(params)
internalScheduler.rescheduleJob(trigger.key, trigger)
}
static unschedule(String triggerName, String triggerGroup =
GrailsJobClassConstants.DEFAULT_TRIGGERS_GROUP) {
+ assertScheduled this.getName()
+ Assert.notNull triggerName, missingArgumentMessage(this.getName(),
'trigger name')
internalScheduler.unscheduleJob(TriggerKey.triggerKey(triggerName,
triggerGroup))
}
@@ -88,6 +106,44 @@ trait QuartzJob implements GrailsApplicationAware {
internalScheduler.scheduleJob(trigger)
}
+ /**
+ * Verifies that the job class has been associated with a scheduler, which
the plugin does for
+ * every enabled job artefact while the application starts.
+ *
+ * @param jobClassName the name of the job class the method was called on
+ */
+ private static void assertScheduled(String jobClassName) {
+ Assert.state internalScheduler != null && internalJobArtefact != null,
+ "The job [${jobClassName}] is not registered with a Quartz
scheduler. Only enabled job " +
+ 'artefacts of a running application are registered, so
check that the plugin is enabled ' +
+ '(quartz.pluginEnabled), that the job is enabled (its
jobEnabled property) and that the ' +
+ 'class is a job artefact of the application.' as String
+ }
+
+ /**
+ * Builds the message reported when a method is called with a null
argument.
+ *
+ * @param jobClassName the name of the job class the method was called on
+ * @param argumentName the name of the argument that is null
+ */
+ private static String missingArgumentMessage(String jobClassName, String
argumentName) {
+ "The ${argumentName} passed for the job [${jobClassName}] is null." as
String
+ }
+
+ /**
+ * Builds the message reported when one of the schedule methods is called
with a null argument. Such a
+ * call resolves to {@link #schedule(Trigger, Map)} unless the caller is
statically compiled, because the
+ * runtime type of null carries no information, hence the hint about the
arguments of the other methods.
+ *
+ * @param jobClassName the name of the job class the method was called on
+ * @param argumentName the name of the argument that is null
+ */
+ private static String missingScheduleArgumentMessage(String jobClassName,
String argumentName) {
+ missingArgumentMessage(jobClassName, argumentName) + ' A null argument
of any of the scheduling ' +
+ 'methods resolves to the one taking a trigger, so also check
the repeat interval, repeat ' +
+ 'count, cron expression and date arguments of the method you
called.'
+ }
+
static setScheduler(Scheduler scheduler) {
internalScheduler = scheduler
}
diff --git
a/grails-quartz/src/test/groovy/grails/plugins/quartz/QuartzJobSpec.groovy
b/grails-quartz/src/test/groovy/grails/plugins/quartz/QuartzJobSpec.groovy
new file mode 100644
index 0000000000..dfb98af009
--- /dev/null
+++ b/grails-quartz/src/test/groovy/grails/plugins/quartz/QuartzJobSpec.groovy
@@ -0,0 +1,325 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package grails.plugins.quartz
+
+import grails.artefact.Artefact
+import groovy.transform.CompileStatic
+import org.quartz.CronTrigger
+import org.quartz.JobDataMap
+import org.quartz.JobKey
+import org.quartz.Scheduler
+import org.quartz.SimpleTrigger
+import org.quartz.Trigger
+import org.quartz.TriggerKey
+import org.quartz.impl.triggers.SimpleTriggerImpl
+import spock.lang.Specification
+import spock.lang.Unroll
+
+/**
+ * Verifies the scheduling methods the {@link QuartzJob} trait adds to every
job artefact.
+ */
+class QuartzJobSpec extends Specification {
+
+ Scheduler scheduler = Mock(Scheduler)
+
+ void setup() {
+ SchedulingJob.scheduler = scheduler
+ SchedulingJob.grailsJobClass = new DefaultGrailsJobClass(SchedulingJob)
+ }
+
+ void cleanup() {
+ SchedulingJob.scheduler = null
+ SchedulingJob.grailsJobClass = null
+ }
+
+ void 'triggerNow fires the job the trait was applied to'() {
+ when:
+ SchedulingJob.triggerNow()
+
+ then:
+ 1 * scheduler.triggerJob(new JobKey(SchedulingJob.name,
'GRAILS_JOBS'), null)
+ }
+
+ void 'triggerNow passes the given parameters as job data'() {
+ when:
+ SchedulingJob.triggerNow(foo: 'bar')
+
+ then:
+ 1 * scheduler.triggerJob(new JobKey(SchedulingJob.name,
'GRAILS_JOBS'), { JobDataMap jobData ->
+ jobData.foo == 'bar'
+ })
+ }
+
+ void 'a repeat interval schedules a simple trigger for the job'() {
+ when:
+ SchedulingJob.schedule(1_000L, 4, [foo: 'bar'])
+
+ then:
+ 1 * scheduler.scheduleJob({ SimpleTrigger trigger ->
+ trigger.jobKey == new JobKey(SchedulingJob.name, 'GRAILS_JOBS')
+ && trigger.key.group == 'GRAILS_TRIGGERS'
+ && trigger.repeatInterval == 1_000L
+ && trigger.repeatCount == 4
+ && trigger.jobDataMap.foo == 'bar'
+ })
+ }
+
+ void 'a repeat interval repeats indefinitely unless a repeat count is
given'() {
+ when:
+ SchedulingJob.schedule(1_000L)
+
+ then:
+ 1 * scheduler.scheduleJob({ SimpleTrigger trigger ->
+ trigger.repeatCount == SimpleTrigger.REPEAT_INDEFINITELY
+ })
+ }
+
+ void 'a date schedules a single execution of the job'() {
+ given:
+ Date scheduleDate = new Date(1_000_000L)
+
+ when:
+ SchedulingJob.schedule(scheduleDate, [foo: 'bar'])
+
+ then:
+ 1 * scheduler.scheduleJob({ Trigger trigger ->
+ trigger.jobKey == new JobKey(SchedulingJob.name, 'GRAILS_JOBS')
+ && trigger.startTime == scheduleDate
+ && trigger.jobDataMap.foo == 'bar'
+ })
+ }
+
+ void 'a cron expression schedules a cron trigger for the job'() {
+ when:
+ SchedulingJob.schedule('0 0 6 * * ?', [foo: 'bar'])
+
+ then:
+ 1 * scheduler.scheduleJob({ CronTrigger trigger ->
+ trigger.jobKey == new JobKey(SchedulingJob.name, 'GRAILS_JOBS')
+ && trigger.cronExpression == '0 0 6 * * ?'
+ && trigger.jobDataMap.foo == 'bar'
+ })
+ }
+
+ void 'a trigger built by the application is re-keyed to the job before it
is scheduled'() {
+ given:
+ SimpleTriggerImpl trigger = new SimpleTriggerImpl('myTrigger',
'myGroup')
+ trigger.jobKey = new JobKey('someOtherJob', 'someOtherGroup')
+
+ when:
+ SchedulingJob.schedule(trigger, [foo: 'bar'])
+
+ then:
+ 1 * scheduler.scheduleJob(trigger)
+ trigger.jobKey == new JobKey(SchedulingJob.name, 'GRAILS_JOBS')
+ trigger.jobDataMap.foo == 'bar'
+ }
+
+ void 'an immutable trigger already keyed to the job is scheduled
unchanged'() {
+ given:
+ JobKey jobKey = new JobKey(SchedulingJob.name, 'GRAILS_JOBS')
+ Trigger trigger = Mock(Trigger) {
+ getJobKey() >> jobKey
+ }
+
+ when:
+ SchedulingJob.schedule(trigger)
+
+ then:
+ 1 * scheduler.scheduleJob(trigger)
+ }
+
+ void 'an immutable trigger keyed to another job is rejected'() {
+ given:
+ Trigger trigger = Mock(Trigger) {
+ getJobKey() >> new JobKey('someOtherJob', 'someOtherGroup')
+ }
+
+ when:
+ SchedulingJob.schedule(trigger)
+
+ then:
+ IllegalArgumentException e = thrown()
+ e.message == 'The trigger job key is not equal to the job key or
the trigger is immutable'
+ 0 * scheduler.scheduleJob(_)
+ }
+
+ void 'rescheduling replaces the trigger registered under its own key'() {
+ given:
+ SimpleTriggerImpl trigger = new SimpleTriggerImpl('myTrigger',
'myGroup')
+
+ when:
+ SchedulingJob.reschedule(trigger, [foo: 'bar'])
+
+ then:
+ 1 * scheduler.rescheduleJob(new TriggerKey('myTrigger',
'myGroup'), trigger)
+ trigger.jobDataMap.foo == 'bar'
+ }
+
+ void 'unscheduling uses the default trigger group unless one is given'() {
+ when:
+ SchedulingJob.unschedule('myTrigger')
+
+ then:
+ 1 * scheduler.unscheduleJob(new TriggerKey('myTrigger',
'GRAILS_TRIGGERS'))
+
+ when:
+ SchedulingJob.unschedule('myTrigger', 'myGroup')
+
+ then:
+ 1 * scheduler.unscheduleJob(new TriggerKey('myTrigger', 'myGroup'))
+ }
+
+ void 'removing the job deletes it from the scheduler'() {
+ when:
+ SchedulingJob.removeJob()
+
+ then:
+ 1 * scheduler.deleteJob(new JobKey(SchedulingJob.name,
'GRAILS_JOBS'))
+ }
+
+ void 'scheduling a job with a null argument reports the trigger as missing
and hints at the other arguments'() {
+ when: 'a null is passed to schedule, which always resolves to the
method taking a trigger'
+ SchedulingJob.schedule(null)
+
+ then:
+ IllegalArgumentException e = thrown()
+ e.message == "The trigger passed for the job
[${SchedulingJob.name}] is null. A null argument of " +
+ 'any of the scheduling methods resolves to the one taking
a trigger, so also check the ' +
+ 'repeat interval, repeat count, cron expression and date
arguments of the method you called.'
+ 0 * scheduler.scheduleJob(_)
+ }
+
+ @Unroll
+ void 'scheduling a job with a null #argument reports which argument is
missing'() {
+ when: 'a statically compiled caller resolves the method by the
declared type of its arguments'
+ invocation.call()
+
+ then:
+ IllegalArgumentException e = thrown()
+ e.message.startsWith("The ${argument} passed for the job
[${SchedulingJob.name}] is null.")
+ 0 * scheduler.scheduleJob(_)
+
+ where:
+ argument | invocation
+ 'repeat interval' | {
StaticallyCompiledScheduler.scheduleWithInterval(null) }
+ 'repeat count' | {
StaticallyCompiledScheduler.scheduleWithRepeatCount(1_000L, null) }
+ 'schedule date' | {
StaticallyCompiledScheduler.scheduleAtDate(null) }
+ 'cron expression' | {
StaticallyCompiledScheduler.scheduleWithCron(null) }
+ 'trigger' | {
StaticallyCompiledScheduler.scheduleWithTrigger(null) }
+ }
+
+ void 'rescheduling a job with a null trigger reports the missing
argument'() {
+ when:
+ SchedulingJob.reschedule(null)
+
+ then:
+ IllegalArgumentException e = thrown()
+ e.message.startsWith("The trigger passed for the job
[${SchedulingJob.name}] is null.")
+ 0 * scheduler.rescheduleJob(_, _)
+ }
+
+ void 'unscheduling a null trigger name reports the missing argument'() {
+ when:
+ SchedulingJob.unschedule(null)
+
+ then:
+ IllegalArgumentException e = thrown()
+ e.message.startsWith("The trigger name passed for the job
[${SchedulingJob.name}] is null.")
+ 0 * scheduler.unscheduleJob(_)
+ }
+
+ @Unroll
+ void 'a job that is not registered with a scheduler reports it instead of
failing with a null pointer'() {
+ when:
+ invocation.call()
+
+ then:
+ IllegalStateException e = thrown()
+ e.message.startsWith("The job [${UnregisteredJob.name}] is not
registered with a Quartz scheduler.")
+
+ where:
+ invocation << [
+ { UnregisteredJob.triggerNow() },
+ { UnregisteredJob.schedule(1_000L) },
+ { UnregisteredJob.schedule(new Date()) },
+ { UnregisteredJob.schedule('0 0 6 * * ?') },
+ { UnregisteredJob.schedule(new
SimpleTriggerImpl('myTrigger', 'myGroup')) },
+ { UnregisteredJob.reschedule(new
SimpleTriggerImpl('myTrigger', 'myGroup')) },
+ { UnregisteredJob.unschedule('myTrigger') },
+ { UnregisteredJob.removeJob() },
+ ]
+ }
+
+ void 'a job whose scheduler is set but which has no artefact is reported
as not registered'() {
+ given:
+ PartiallyRegisteredJob.scheduler = scheduler
+
+ when:
+ PartiallyRegisteredJob.triggerNow()
+
+ then:
+ IllegalStateException e = thrown()
+ e.message.startsWith("The job [${PartiallyRegisteredJob.name}] is
not registered with a Quartz scheduler.")
+ 0 * scheduler.triggerJob(_, _)
+
+ cleanup:
+ PartiallyRegisteredJob.scheduler = null
+ }
+}
+
+@CompileStatic
+class StaticallyCompiledScheduler {
+
+ static void scheduleWithInterval(Long repeatInterval) {
+ SchedulingJob.schedule(repeatInterval)
+ }
+
+ static void scheduleWithRepeatCount(Long repeatInterval, Integer
repeatCount) {
+ SchedulingJob.schedule(repeatInterval, repeatCount)
+ }
+
+ static void scheduleAtDate(Date scheduleDate) {
+ SchedulingJob.schedule(scheduleDate)
+ }
+
+ static void scheduleWithCron(String cronExpression) {
+ SchedulingJob.schedule(cronExpression)
+ }
+
+ static void scheduleWithTrigger(Trigger trigger) {
+ SchedulingJob.schedule(trigger)
+ }
+}
+
+@Artefact('Job')
+class SchedulingJob {
+ void execute() {}
+}
+
+@Artefact('Job')
+class UnregisteredJob {
+ void execute() {}
+}
+
+@Artefact('Job')
+class PartiallyRegisteredJob {
+ void execute() {}
+}
diff --git
a/grails-test-examples/quartz/grails-app/jobs/quartzapp/DisabledJob.groovy
b/grails-test-examples/quartz/grails-app/jobs/quartzapp/DisabledJob.groovy
new file mode 100644
index 0000000000..523c66be97
--- /dev/null
+++ b/grails-test-examples/quartz/grails-app/jobs/quartzapp/DisabledJob.groovy
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package quartzapp
+
+/**
+ * A job that is turned off, so that it never reaches the scheduler and the
scheduling methods of the
+ * QuartzJob trait have nothing to work with.
+ */
+class DisabledJob {
+
+ static jobEnabled = false
+
+ void execute() {}
+}
diff --git
a/grails-test-examples/quartz/src/integration-test/groovy/quartzapp/QuartzSchedulingSpec.groovy
b/grails-test-examples/quartz/src/integration-test/groovy/quartzapp/QuartzSchedulingSpec.groovy
index 3114876364..b8c39ed3f1 100644
---
a/grails-test-examples/quartz/src/integration-test/groovy/quartzapp/QuartzSchedulingSpec.groovy
+++
b/grails-test-examples/quartz/src/integration-test/groovy/quartzapp/QuartzSchedulingSpec.groovy
@@ -83,4 +83,52 @@ class QuartzSchedulingSpec extends Specification {
assert OnDemandJob.EXECUTIONS.get() > before
}
}
+
+ void 'a job scheduled at runtime with a repeat interval is executed by the
scheduler'() {
+ given:
+ PollingConditions conditions = new PollingConditions(timeout: 30)
+ int before = OnDemandJob.EXECUTIONS.get()
+
+ when:
+ OnDemandJob.schedule(200L, 2, [foo: 'bar'])
+
+ then:
+ conditions.eventually {
+ assert OnDemandJob.EXECUTIONS.get() > before
+ }
+ }
+
+ void 'a job scheduled at runtime with a cron expression is registered with
the scheduler'() {
+ given:
+ JobKey jobKey = JobKey.jobKey(OnDemandJob.name, 'GRAILS_JOBS')
+ int before = quartzScheduler.getTriggersOfJob(jobKey).size()
+
+ when:
+ OnDemandJob.schedule('0 0 6 * * ?')
+
+ then:
+ quartzScheduler.getTriggersOfJob(jobKey).size() == before + 1
+ }
+
+ void 'scheduling a job with a null argument reports the null instead of
failing with a null pointer'() {
+ when: 'the value an application passes to schedule turns out to be
null'
+ OnDemandJob.schedule(null)
+
+ then:
+ IllegalArgumentException e = thrown()
+ e.message.startsWith("The trigger passed for the job
[${OnDemandJob.name}] is null.")
+ e.message.contains('resolves to the one taking a trigger')
+ }
+
+ void 'scheduling a job that the scheduler does not know about reports
why'() {
+ when: 'a job that is turned off is scheduled at runtime'
+ DisabledJob.triggerNow()
+
+ then:
+ IllegalStateException e = thrown()
+ e.message.startsWith("The job [${DisabledJob.name}] is not
registered with a Quartz scheduler.")
+
+ and: 'it never reached the scheduler in the first place'
+ !quartzScheduler.checkExists(JobKey.jobKey(DisabledJob.name,
'GRAILS_JOBS'))
+ }
}
