Deprecate default constructor on ConcurrentTaskExecutor/Scheduler

Includes revision of null Executor configuration.
Respects TaskDecorator configuration on DefaultManagedTaskExecutor.

Closes gh-27914

Author: jhoeller

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

@@ -65,6 +65,10 @@
 @SuppressWarnings("deprecation")
 public class ConcurrentTaskExecutor implements AsyncListenableTaskExecutor, SchedulingTaskExecutor {
 
+	private static final Executor STUB_EXECUTOR = (task -> {
+		throw new IllegalStateException("Executor not configured");
+	});
+
 	@Nullable
 	private static Class<?> managedExecutorServiceClass;
 
@@ -80,15 +84,22 @@ public class ConcurrentTaskExecutor implements AsyncListenableTaskExecutor, Sche
 		}
 	}
 
-	private Executor concurrentExecutor;
 
-	private TaskExecutorAdapter adaptedExecutor;
+	private Executor concurrentExecutor = STUB_EXECUTOR;
+
+	private TaskExecutorAdapter adaptedExecutor = new TaskExecutorAdapter(STUB_EXECUTOR);
+
+	@Nullable
+	private TaskDecorator taskDecorator;
 
 
 	/**
 	 * Create a new ConcurrentTaskExecutor, using a single thread executor as default.
 	 * @see java.util.concurrent.Executors#newSingleThreadExecutor()
+	 * @deprecated in favor of {@link #ConcurrentTaskExecutor(Executor)} with an
+	 * externally provided Executor
 	 */
+	@Deprecated(since = "6.1")
 	public ConcurrentTaskExecutor() {
 		this.concurrentExecutor = Executors.newSingleThreadExecutor();
 		this.adaptedExecutor = new TaskExecutorAdapter(this.concurrentExecutor);
@@ -101,8 +112,9 @@ public ConcurrentTaskExecutor() {
 	 * @param executor the {@link java.util.concurrent.Executor} to delegate to
 	 */
 	public ConcurrentTaskExecutor(@Nullable Executor executor) {
-		this.concurrentExecutor = (executor != null ? executor : Executors.newSingleThreadExecutor());
-		this.adaptedExecutor = getAdaptedExecutor(this.concurrentExecutor);
+		if (executor != null) {
+			setConcurrentExecutor(executor);
+		}
 	}
 
 
@@ -111,8 +123,8 @@ public ConcurrentTaskExecutor(@Nullable Executor executor) {
 	 * <p>Autodetects a JSR-236 {@link jakarta.enterprise.concurrent.ManagedExecutorService}
 	 * in order to expose {@link jakarta.enterprise.concurrent.ManagedTask} adapters for it.
 	 */
-	public final void setConcurrentExecutor(@Nullable Executor executor) {
-		this.concurrentExecutor = (executor != null ? executor : Executors.newSingleThreadExecutor());
+	public final void setConcurrentExecutor(Executor executor) {
+		this.concurrentExecutor = executor;
 		this.adaptedExecutor = getAdaptedExecutor(this.concurrentExecutor);
 	}
 
@@ -139,6 +151,7 @@ public final Executor getConcurrentExecutor() {
 	 * @since 4.3
 	 */
 	public final void setTaskDecorator(TaskDecorator taskDecorator) {
+		this.taskDecorator = taskDecorator;
 		this.adaptedExecutor.setTaskDecorator(taskDecorator);
 	}
 
@@ -175,11 +188,15 @@ public <T> ListenableFuture<T> submitListenable(Callable<T> task) {
 	}
 
 
-	private static TaskExecutorAdapter getAdaptedExecutor(Executor concurrentExecutor) {
+	private TaskExecutorAdapter getAdaptedExecutor(Executor concurrentExecutor) {
 		if (managedExecutorServiceClass != null && managedExecutorServiceClass.isInstance(concurrentExecutor)) {
 			return new ManagedTaskExecutorAdapter(concurrentExecutor);
 		}
-		return new TaskExecutorAdapter(concurrentExecutor);
+		TaskExecutorAdapter adapter = new TaskExecutorAdapter(concurrentExecutor);
+		if (this.taskDecorator != null) {
+			adapter.setTaskDecorator(this.taskDecorator);
+		}
+		return adapter;
 	}
 
 

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

@@ -100,10 +100,14 @@ public class ConcurrentTaskScheduler extends ConcurrentTaskExecutor implements T
 	 * Create a new ConcurrentTaskScheduler,
 	 * using a single thread executor as default.
 	 * @see java.util.concurrent.Executors#newSingleThreadScheduledExecutor()
+	 * @deprecated in favor of {@link #ConcurrentTaskScheduler(ScheduledExecutorService)}
+	 * with an externally provided Executor
 	 */
+	@Deprecated(since = "6.1")
 	public ConcurrentTaskScheduler() {
 		super();
-		this.scheduledExecutor = initScheduledExecutor(null);
+		this.scheduledExecutor = Executors.newSingleThreadScheduledExecutor();
+		this.enterpriseConcurrentScheduler = false;
 	}
 
 	/**
@@ -116,9 +120,11 @@ public ConcurrentTaskScheduler() {
 	 * to delegate to for {@link org.springframework.scheduling.SchedulingTaskExecutor}
 	 * as well as {@link TaskScheduler} invocations
 	 */
-	public ConcurrentTaskScheduler(ScheduledExecutorService scheduledExecutor) {
+	public ConcurrentTaskScheduler(@Nullable ScheduledExecutorService scheduledExecutor) {
 		super(scheduledExecutor);
-		this.scheduledExecutor = initScheduledExecutor(scheduledExecutor);
+		if (scheduledExecutor != null) {
+			initScheduledExecutor(scheduledExecutor);
+		}
 	}
 
 	/**
@@ -134,21 +140,14 @@ public ConcurrentTaskScheduler(ScheduledExecutorService scheduledExecutor) {
 	 */
 	public ConcurrentTaskScheduler(Executor concurrentExecutor, ScheduledExecutorService scheduledExecutor) {
 		super(concurrentExecutor);
-		this.scheduledExecutor = initScheduledExecutor(scheduledExecutor);
+		initScheduledExecutor(scheduledExecutor);
 	}
 
 
-	private ScheduledExecutorService initScheduledExecutor(@Nullable ScheduledExecutorService scheduledExecutor) {
-		if (scheduledExecutor != null) {
-			this.scheduledExecutor = scheduledExecutor;
-			this.enterpriseConcurrentScheduler = (managedScheduledExecutorServiceClass != null &&
-					managedScheduledExecutorServiceClass.isInstance(scheduledExecutor));
-		}
-		else {
-			this.scheduledExecutor = Executors.newSingleThreadScheduledExecutor();
-			this.enterpriseConcurrentScheduler = false;
-		}
-		return this.scheduledExecutor;
+	private void initScheduledExecutor(ScheduledExecutorService scheduledExecutor) {
+		this.scheduledExecutor = scheduledExecutor;
+		this.enterpriseConcurrentScheduler = (managedScheduledExecutorServiceClass != null &&
+				managedScheduledExecutorServiceClass.isInstance(scheduledExecutor));
 	}
 
 	/**
@@ -162,7 +161,7 @@ private ScheduledExecutorService initScheduledExecutor(@Nullable ScheduledExecut
 	 * as well, pass the same executor reference to {@link #setConcurrentExecutor}.
 	 * @see #setConcurrentExecutor
 	 */
-	public void setScheduledExecutor(@Nullable ScheduledExecutorService scheduledExecutor) {
+	public void setScheduledExecutor(ScheduledExecutorService scheduledExecutor) {
 		initScheduledExecutor(scheduledExecutor);
 	}
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * 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.
@@ -24,7 +24,6 @@
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.jndi.JndiLocatorDelegate;
 import org.springframework.jndi.JndiTemplate;
-import org.springframework.lang.Nullable;
 
 /**
  * JNDI-based variant of {@link ConcurrentTaskExecutor}, performing a default lookup for
@@ -43,10 +42,15 @@ public class DefaultManagedTaskExecutor extends ConcurrentTaskExecutor implement
 
 	private final JndiLocatorDelegate jndiLocator = new JndiLocatorDelegate();
 
-	@Nullable
 	private String jndiName = "java:comp/DefaultManagedExecutorService";
 
 
+	public DefaultManagedTaskExecutor() {
+		// Executor initialization happens in afterPropertiesSet
+		super(null);
+	}
+
+
 	/**
 	 * Set the JNDI template to use for JNDI lookups.
 	 * @see org.springframework.jndi.JndiAccessor#setJndiTemplate
@@ -87,9 +91,7 @@ public void setJndiName(String jndiName) {
 
 	@Override
 	public void afterPropertiesSet() throws NamingException {
-		if (this.jndiName != null) {
-			setConcurrentExecutor(this.jndiLocator.lookup(this.jndiName, Executor.class));
-		}
+		setConcurrentExecutor(this.jndiLocator.lookup(this.jndiName, Executor.class));
 	}
 
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * 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.
@@ -24,7 +24,6 @@
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.jndi.JndiLocatorDelegate;
 import org.springframework.jndi.JndiTemplate;
-import org.springframework.lang.Nullable;
 
 /**
  * JNDI-based variant of {@link ConcurrentTaskScheduler}, performing a default lookup for
@@ -43,10 +42,15 @@ public class DefaultManagedTaskScheduler extends ConcurrentTaskScheduler impleme
 
 	private final JndiLocatorDelegate jndiLocator = new JndiLocatorDelegate();
 
-	@Nullable
 	private String jndiName = "java:comp/DefaultManagedScheduledExecutorService";
 
 
+	public DefaultManagedTaskScheduler() {
+		// Executor initialization happens in afterPropertiesSet
+		super(null);
+	}
+
+
 	/**
 	 * Set the JNDI template to use for JNDI lookups.
 	 * @see org.springframework.jndi.JndiAccessor#setJndiTemplate
@@ -87,11 +91,9 @@ public void setJndiName(String jndiName) {
 
 	@Override
 	public void afterPropertiesSet() throws NamingException {
-		if (this.jndiName != null) {
-			ScheduledExecutorService executor = this.jndiLocator.lookup(this.jndiName, ScheduledExecutorService.class);
-			setConcurrentExecutor(executor);
-			setScheduledExecutor(executor);
-		}
+		ScheduledExecutorService executor = this.jndiLocator.lookup(this.jndiName, ScheduledExecutorService.class);
+		setConcurrentExecutor(executor);
+		setScheduledExecutor(executor);
 	}
 
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2022 the original author or authors.
+ * 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.
@@ -16,6 +16,7 @@
 
 package org.springframework.scheduling.concurrent;
 
+import java.util.concurrent.Executor;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.RunnableFuture;
 import java.util.concurrent.ThreadPoolExecutor;
@@ -25,6 +26,8 @@
 import org.junit.jupiter.api.Test;
 
 import org.springframework.core.task.NoOpRunnable;
+import org.springframework.core.task.TaskDecorator;
+import org.springframework.util.Assert;
 
 import static org.assertj.core.api.Assertions.assertThatCode;
 
@@ -38,8 +41,8 @@ class ConcurrentTaskExecutorTests extends AbstractSchedulingTaskExecutorTests {
 			new ThreadPoolExecutor(1, 1, 60, TimeUnit.SECONDS, new LinkedBlockingQueue<>());
 
 
-	@Override
 	@SuppressWarnings("deprecation")
+	@Override
 	protected org.springframework.core.task.AsyncListenableTaskExecutor buildExecutor() {
 		concurrentExecutor.setThreadFactory(new CustomizableThreadFactory(this.threadNamePrefix));
 		return new ConcurrentTaskExecutor(concurrentExecutor);
@@ -65,14 +68,44 @@ void zeroArgCtorResultsInDefaultTaskExecutorBeingUsed() {
 	@Test
 	void passingNullExecutorToCtorResultsInDefaultTaskExecutorBeingUsed() {
 		ConcurrentTaskExecutor executor = new ConcurrentTaskExecutor(null);
+		assertThatCode(() -> executor.execute(new NoOpRunnable())).hasMessage("Executor not configured");
+	}
+
+	@Test
+	void earlySetConcurrentExecutorCallRespectsConfiguredTaskDecorator() {
+		ConcurrentTaskExecutor executor = new ConcurrentTaskExecutor();
+		executor.setConcurrentExecutor(new DecoratedExecutor());
+		executor.setTaskDecorator(new RunnableDecorator());
 		assertThatCode(() -> executor.execute(new NoOpRunnable())).doesNotThrowAnyException();
 	}
 
 	@Test
-	void passingNullExecutorToSetterResultsInDefaultTaskExecutorBeingUsed() {
+	void lateSetConcurrentExecutorCallRespectsConfiguredTaskDecorator() {
 		ConcurrentTaskExecutor executor = new ConcurrentTaskExecutor();
-		executor.setConcurrentExecutor(null);
+		executor.setTaskDecorator(new RunnableDecorator());
+		executor.setConcurrentExecutor(new DecoratedExecutor());
 		assertThatCode(() -> executor.execute(new NoOpRunnable())).doesNotThrowAnyException();
 	}
 
+
+	private static class DecoratedRunnable implements Runnable {
+		@Override
+		public void run() {
+		}
+	}
+
+	private static class RunnableDecorator implements TaskDecorator {
+		@Override
+		public Runnable decorate(Runnable runnable) {
+			return new DecoratedRunnable();
+		}
+	}
+
+	private static class DecoratedExecutor implements Executor {
+		@Override
+		public void execute(Runnable command) {
+			Assert.state(command instanceof DecoratedRunnable, "TaskDecorator not applied");
+		}
+	}
+
 }