Provide a dedicated exception if ScheduledExecutor is not configured

While it is not easily possible to create a ConcurrentTaskScheduler
with no scheduled executor, DefaultManagedTaskScheduler has a default
constructor that does that as the JNDI lookup happens in the regular
afterPropertiesSet callback. If such an instance is created manually,
it can throw an unhelpful NullPointerException if one attempts to
schedule a task.

This commit updates ConcurrentTaskScheduler to flag that the scheduled
executor could be `null` and check if that's the case before using it.
This now throws a dedicated exception that should hopefully provide
better context for those upgrading.

See gh-27914

Closes gh-31751

Author: snicoll

spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskScheduler.java

@@ -89,6 +89,7 @@ public class ConcurrentTaskScheduler extends ConcurrentTaskExecutor implements T
 	}
 
 
+	@Nullable
 	private ScheduledExecutorService scheduledExecutor;
 
 	private boolean enterpriseConcurrentScheduler = false;
@@ -168,6 +169,13 @@ public void setScheduledExecutor(ScheduledExecutorService scheduledExecutor) {
 		initScheduledExecutor(scheduledExecutor);
 	}
 
+	private ScheduledExecutorService getScheduledExecutor() {
+		if (this.scheduledExecutor == null) {
+			throw new IllegalStateException("No ScheduledExecutor is configured");
+		}
+		return this.scheduledExecutor;
+	}
+
 	/**
 	 * Provide an {@link ErrorHandler} strategy.
 	 */
@@ -195,75 +203,81 @@ public Clock getClock() {
 	@Override
 	@Nullable
 	public ScheduledFuture<?> schedule(Runnable task, Trigger trigger) {
+		ScheduledExecutorService scheduleExecutorToUse = getScheduledExecutor();
 		try {
 			if (this.enterpriseConcurrentScheduler) {
 				return new EnterpriseConcurrentTriggerScheduler().schedule(decorateTask(task, true), trigger);
 			}
 			else {
 				ErrorHandler errorHandler =
 						(this.errorHandler != null ? this.errorHandler : TaskUtils.getDefaultErrorHandler(true));
-				return new ReschedulingRunnable(task, trigger, this.clock, this.scheduledExecutor, errorHandler).schedule();
+				return new ReschedulingRunnable(task, trigger, this.clock, scheduleExecutorToUse, errorHandler).schedule();
 			}
 		}
 		catch (RejectedExecutionException ex) {
-			throw new TaskRejectedException(this.scheduledExecutor, task, ex);
+			throw new TaskRejectedException(scheduleExecutorToUse, task, ex);
 		}
 	}
 
 	@Override
 	public ScheduledFuture<?> schedule(Runnable task, Instant startTime) {
+		ScheduledExecutorService scheduleExecutorToUse = getScheduledExecutor();
 		Duration delay = Duration.between(this.clock.instant(), startTime);
 		try {
-			return this.scheduledExecutor.schedule(decorateTask(task, false), NANO.convert(delay), NANO);
+			return scheduleExecutorToUse.schedule(decorateTask(task, false), NANO.convert(delay), NANO);
 		}
 		catch (RejectedExecutionException ex) {
-			throw new TaskRejectedException(this.scheduledExecutor, task, ex);
+			throw new TaskRejectedException(scheduleExecutorToUse, task, ex);
 		}
 	}
 
 	@Override
 	public ScheduledFuture<?> scheduleAtFixedRate(Runnable task, Instant startTime, Duration period) {
+		ScheduledExecutorService scheduleExecutorToUse = getScheduledExecutor();
 		Duration initialDelay = Duration.between(this.clock.instant(), startTime);
 		try {
-			return this.scheduledExecutor.scheduleAtFixedRate(decorateTask(task, true),
+			return scheduleExecutorToUse.scheduleAtFixedRate(decorateTask(task, true),
 					NANO.convert(initialDelay), NANO.convert(period), NANO);
 		}
 		catch (RejectedExecutionException ex) {
-			throw new TaskRejectedException(this.scheduledExecutor, task, ex);
+			throw new TaskRejectedException(scheduleExecutorToUse, task, ex);
 		}
 	}
 
 	@Override
 	public ScheduledFuture<?> scheduleAtFixedRate(Runnable task, Duration period) {
+		ScheduledExecutorService scheduleExecutorToUse = getScheduledExecutor();
 		try {
-			return this.scheduledExecutor.scheduleAtFixedRate(decorateTask(task, true),
+			return scheduleExecutorToUse.scheduleAtFixedRate(decorateTask(task, true),
 					0, NANO.convert(period), NANO);
 		}
 		catch (RejectedExecutionException ex) {
-			throw new TaskRejectedException(this.scheduledExecutor, task, ex);
+			throw new TaskRejectedException(scheduleExecutorToUse, task, ex);
 		}
 	}
 
 	@Override
 	public ScheduledFuture<?> scheduleWithFixedDelay(Runnable task, Instant startTime, Duration delay) {
+		ScheduledExecutorService scheduleExecutorToUse = getScheduledExecutor();
 		Duration initialDelay = Duration.between(this.clock.instant(), startTime);
 		try {
-			return this.scheduledExecutor.scheduleWithFixedDelay(decorateTask(task, true),
+			return scheduleExecutorToUse.scheduleWithFixedDelay(decorateTask(task, true),
 					NANO.convert(initialDelay), NANO.convert(delay), NANO);
 		}
 		catch (RejectedExecutionException ex) {
-			throw new TaskRejectedException(this.scheduledExecutor, task, ex);
+			throw new TaskRejectedException(scheduleExecutorToUse, task, ex);
 		}
 	}
 
 	@Override
 	public ScheduledFuture<?> scheduleWithFixedDelay(Runnable task, Duration delay) {
+		ScheduledExecutorService scheduleExecutorToUse = getScheduledExecutor();
 		try {
-			return this.scheduledExecutor.scheduleWithFixedDelay(decorateTask(task, true),
+			return scheduleExecutorToUse.scheduleWithFixedDelay(decorateTask(task, true),
 					0, NANO.convert(delay), NANO);
 		}
 		catch (RejectedExecutionException ex) {
-			throw new TaskRejectedException(this.scheduledExecutor, task, ex);
+			throw new TaskRejectedException(scheduleExecutorToUse, task, ex);
 		}
 	}
 
@@ -283,7 +297,7 @@ private Runnable decorateTask(Runnable task, boolean isRepeatingTask) {
 	private class EnterpriseConcurrentTriggerScheduler {
 
 		public ScheduledFuture<?> schedule(Runnable task, Trigger trigger) {
-			ManagedScheduledExecutorService executor = (ManagedScheduledExecutorService) scheduledExecutor;
+			ManagedScheduledExecutorService executor = (ManagedScheduledExecutorService) getScheduledExecutor();
 			return executor.schedule(task, new TriggerAdapter(trigger));
 		}
 

spring-context/src/main/java/org/springframework/scheduling/concurrent/DefaultManagedTaskScheduler.java

@@ -28,6 +28,8 @@
 /**
  * JNDI-based variant of {@link ConcurrentTaskScheduler}, performing a default lookup for
  * JSR-236's "java:comp/DefaultManagedScheduledExecutorService" in a Jakarta EE environment.
+ * Expected to be exposed as a bean, in particular as the default lookup happens in the
+ * standard {@link InitializingBean#afterPropertiesSet()} callback.
  *
  * <p>Note: This class is not strictly JSR-236 based; it can work with any regular
  * {@link java.util.concurrent.ScheduledExecutorService} that can be found in JNDI.

spring-context/src/test/java/org/springframework/scheduling/concurrent/DefaultManagedTaskSchedulerTests.java

@@ -0,0 +1,86 @@
+/*
+ * Copyright 2002-2023 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.
+ * 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 org.springframework.scheduling.concurrent;
+
+import java.time.Duration;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+
+import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
+import org.junit.jupiter.api.Test;
+
+import org.springframework.scheduling.Trigger;
+
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.mock;
+
+/**
+ * Tests for {@link DefaultManagedTaskScheduler}.
+ *
+ * @author Stephane Nicoll
+ */
+class DefaultManagedTaskSchedulerTests {
+
+	private final Runnable NO_OP = () -> {};
+
+	@Test
+	void scheduleWithTriggerAndNoScheduledExecutorProvidesDedicatedException() {
+		DefaultManagedTaskScheduler scheduler = new DefaultManagedTaskScheduler();
+		assertNoExecutorException(() -> scheduler.schedule(NO_OP, mock(Trigger.class)));
+	}
+
+	@Test
+	void scheduleWithInstantAndNoScheduledExecutorProvidesDedicatedException() {
+		DefaultManagedTaskScheduler scheduler = new DefaultManagedTaskScheduler();
+		assertNoExecutorException(() -> scheduler.schedule(NO_OP, Instant.now()));
+	}
+
+	@Test
+	void scheduleAtFixedRateWithStartTimeAndDurationAndNoScheduledExecutorProvidesDedicatedException() {
+		DefaultManagedTaskScheduler scheduler = new DefaultManagedTaskScheduler();
+		assertNoExecutorException(() -> scheduler.scheduleAtFixedRate(
+				NO_OP, Instant.now(), Duration.of(1, ChronoUnit.MINUTES)));
+	}
+
+	@Test
+	void scheduleAtFixedRateWithDurationAndNoScheduledExecutorProvidesDedicatedException() {
+		DefaultManagedTaskScheduler scheduler = new DefaultManagedTaskScheduler();
+		assertNoExecutorException(() -> scheduler.scheduleAtFixedRate(
+				NO_OP, Duration.of(1, ChronoUnit.MINUTES)));
+	}
+
+	@Test
+	void scheduleWithFixedDelayWithStartTimeAndDurationAndNoScheduledExecutorProvidesDedicatedException() {
+		DefaultManagedTaskScheduler scheduler = new DefaultManagedTaskScheduler();
+		assertNoExecutorException(() -> scheduler.scheduleWithFixedDelay(
+				NO_OP, Instant.now(), Duration.of(1, ChronoUnit.MINUTES)));
+	}
+
+	@Test
+	void scheduleWithFixedDelayWithDurationAndNoScheduledExecutorProvidesDedicatedException() {
+		DefaultManagedTaskScheduler scheduler = new DefaultManagedTaskScheduler();
+		assertNoExecutorException(() -> scheduler.scheduleWithFixedDelay(
+				NO_OP, Duration.of(1, ChronoUnit.MINUTES)));
+	}
+
+	private void assertNoExecutorException(ThrowingCallable callable) {
+		assertThatThrownBy(callable)
+				.isInstanceOf(IllegalStateException.class)
+				.hasMessage("No ScheduledExecutor is configured");
+	}
+
+}