Support disabling cron jobs registered via SchedulingConfigurer

Prior to this commit, support was provided for disabling cron jobs
configured with an explicit "-" cron expression. However, the "-"
expression was only supported when supplied via the @scheduled
annotation.

This commit adds support for disabling cron jobs configured with the
"-" cron expression when a cron job is registered via the
addCronTask(Runnable, String) method in the ScheduledTaskRegistrar
supplied to a SchedulingConfigurer.

Closes gh-23568

spring-context/src/main/java/org/springframework/scheduling/config/ScheduledTaskRegistrar.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -42,20 +42,33 @@
  * expressions.
  *
  * <p>As of Spring 3.1, {@code ScheduledTaskRegistrar} has a more prominent user-facing
- * role when used in conjunction with the @{@link
- * org.springframework.scheduling.annotation.EnableAsync EnableAsync} annotation and its
+ * role when used in conjunction with the {@link
+ * org.springframework.scheduling.annotation.EnableAsync @EnableAsync} annotation and its
  * {@link org.springframework.scheduling.annotation.SchedulingConfigurer
  * SchedulingConfigurer} callback interface.
  *
  * @author Juergen Hoeller
  * @author Chris Beams
  * @author Tobias Montagna-Hay
+ * @author Sam Brannen
  * @since 3.0
  * @see org.springframework.scheduling.annotation.EnableAsync
  * @see org.springframework.scheduling.annotation.SchedulingConfigurer
  */
 public class ScheduledTaskRegistrar implements ScheduledTaskHolder, InitializingBean, DisposableBean {
 
+	/**
+	 * A special cron expression value that indicates a disabled trigger: {@value}.
+	 * <p>This is primarily meant for use with {@link #addCronTask(Runnable, String)}
+	 * when the value for the supplied {@code expression} is retrieved from an
+	 * external source &mdash; for example, from a property in the
+	 * {@link org.springframework.core.env.Environment Environment}.
+	 * @since 5.2
+	 * @see org.springframework.scheduling.annotation.Scheduled#CRON_DISABLED
+	 */
+	public static final String CRON_DISABLED = "-";
+
+
 	@Nullable
 	private TaskScheduler taskScheduler;
 
@@ -254,10 +267,14 @@ public void addTriggerTask(TriggerTask task) {
 	}
 
 	/**
-	 * Add a Runnable task to be triggered per the given cron expression.
+	 * Add a {@link Runnable} task to be triggered per the given cron {@code expression}.
+	 * <p>As of Spring Framework 5.2, this method will not register the task if the
+	 * {@code expression} is equal to {@link #CRON_DISABLED}.
 	 */
 	public void addCronTask(Runnable task, String expression) {
-		addCronTask(new CronTask(task, expression));
+		if (!CRON_DISABLED.equals(expression)) {
+			addCronTask(new CronTask(task, expression));
+		}
 	}
 
 	/**

spring-context/src/test/java/org/springframework/scheduling/config/ScheduledTaskRegistrarTests.java

@@ -17,71 +17,82 @@
 package org.springframework.scheduling.config;
 
 import java.util.Collections;
-import java.util.List;
 
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.mockito.Mockito.mock;
 
 /**
  * Unit tests for {@link ScheduledTaskRegistrar}.
  *
  * @author Tobias Montagna-Hay
  * @author Juergen Hoeller
+ * @author Sam Brannen
  * @since 4.2
  */
-public class ScheduledTaskRegistrarTests {
+class ScheduledTaskRegistrarTests {
+
+	private static final Runnable no_op = () -> {};
 
 	private final ScheduledTaskRegistrar taskRegistrar = new ScheduledTaskRegistrar();
 
 
-	@Test
-	public void emptyTaskLists() {
-		assertThat(this.taskRegistrar.getTriggerTaskList().isEmpty()).isTrue();
-		assertThat(this.taskRegistrar.getCronTaskList().isEmpty()).isTrue();
-		assertThat(this.taskRegistrar.getFixedRateTaskList().isEmpty()).isTrue();
-		assertThat(this.taskRegistrar.getFixedDelayTaskList().isEmpty()).isTrue();
+	@BeforeEach
+	void preconditions() {
+		assertThat(this.taskRegistrar.getTriggerTaskList()).isEmpty();
+		assertThat(this.taskRegistrar.getCronTaskList()).isEmpty();
+		assertThat(this.taskRegistrar.getFixedRateTaskList()).isEmpty();
+		assertThat(this.taskRegistrar.getFixedDelayTaskList()).isEmpty();
 	}
 
 	@Test
-	public void getTriggerTasks() {
+	void getTriggerTasks() {
 		TriggerTask mockTriggerTask = mock(TriggerTask.class);
-		List<TriggerTask> triggerTaskList = Collections.singletonList(mockTriggerTask);
-		this.taskRegistrar.setTriggerTasksList(triggerTaskList);
-		List<TriggerTask> retrievedList = this.taskRegistrar.getTriggerTaskList();
-		assertThat(retrievedList.size()).isEqualTo(1);
-		assertThat(retrievedList.get(0)).isEqualTo(mockTriggerTask);
+		this.taskRegistrar.setTriggerTasksList(Collections.singletonList(mockTriggerTask));
+		assertThat(this.taskRegistrar.getTriggerTaskList()).containsExactly(mockTriggerTask);
 	}
 
 	@Test
-	public void getCronTasks() {
+	void getCronTasks() {
 		CronTask mockCronTask = mock(CronTask.class);
-		List<CronTask> cronTaskList = Collections.singletonList(mockCronTask);
-		this.taskRegistrar.setCronTasksList(cronTaskList);
-		List<CronTask> retrievedList = this.taskRegistrar.getCronTaskList();
-		assertThat(retrievedList.size()).isEqualTo(1);
-		assertThat(retrievedList.get(0)).isEqualTo(mockCronTask);
+		this.taskRegistrar.setCronTasksList(Collections.singletonList(mockCronTask));
+		assertThat(this.taskRegistrar.getCronTaskList()).containsExactly(mockCronTask);
 	}
 
 	@Test
-	public void getFixedRateTasks() {
+	void getFixedRateTasks() {
 		IntervalTask mockFixedRateTask = mock(IntervalTask.class);
-		List<IntervalTask> fixedRateTaskList = Collections.singletonList(mockFixedRateTask);
-		this.taskRegistrar.setFixedRateTasksList(fixedRateTaskList);
-		List<IntervalTask> retrievedList = this.taskRegistrar.getFixedRateTaskList();
-		assertThat(retrievedList.size()).isEqualTo(1);
-		assertThat(retrievedList.get(0)).isEqualTo(mockFixedRateTask);
+		this.taskRegistrar.setFixedRateTasksList(Collections.singletonList(mockFixedRateTask));
+		assertThat(this.taskRegistrar.getFixedRateTaskList()).containsExactly(mockFixedRateTask);
 	}
 
 	@Test
-	public void getFixedDelayTasks() {
+	void getFixedDelayTasks() {
 		IntervalTask mockFixedDelayTask = mock(IntervalTask.class);
-		List<IntervalTask> fixedDelayTaskList = Collections.singletonList(mockFixedDelayTask);
-		this.taskRegistrar.setFixedDelayTasksList(fixedDelayTaskList);
-		List<IntervalTask> retrievedList = this.taskRegistrar.getFixedDelayTaskList();
-		assertThat(retrievedList.size()).isEqualTo(1);
-		assertThat(retrievedList.get(0)).isEqualTo(mockFixedDelayTask);
+		this.taskRegistrar.setFixedDelayTasksList(Collections.singletonList(mockFixedDelayTask));
+		assertThat(this.taskRegistrar.getFixedDelayTaskList()).containsExactly(mockFixedDelayTask);
+	}
+
+	@Test
+	void addCronTaskWithValidExpression() {
+		this.taskRegistrar.addCronTask(no_op, "* * * * * ?");
+		assertThat(this.taskRegistrar.getCronTaskList()).size().isEqualTo(1);
+	}
+
+	@Test
+	void addCronTaskWithInvalidExpression() {
+		assertThatIllegalArgumentException()
+			.isThrownBy(() -> this.taskRegistrar.addCronTask(no_op, "* * *"))
+			.withMessage("Cron expression must consist of 6 fields (found 3 in \"* * *\")");
+	}
+
+	@Test
+	void addCronTaskWithDisabledExpression() {
+		this.taskRegistrar.addCronTask(no_op, ScheduledTaskRegistrar.CRON_DISABLED);
+		assertThat(this.taskRegistrar.getCronTaskList()).isEmpty();
 	}
 
 }