From 2c7e8661cfa88afcf75a9d0460ec867e7318d019 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 8 May 2023 12:02:25 +0200 Subject: [PATCH] Respect TaskDecorator configuration on DefaultManagedTaskExecutor Closes gh-30442 --- .../concurrent/ConcurrentTaskExecutor.java | 12 +++++- .../concurrent/ConcurrentTaskScheduler.java | 9 ++-- .../ConcurrentTaskExecutorTests.java | 41 +++++++++++++++++-- 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskExecutor.java b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskExecutor.java index 17301854989..f0fd31a8abd 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskExecutor.java +++ b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskExecutor.java @@ -84,6 +84,9 @@ public class ConcurrentTaskExecutor implements AsyncListenableTaskExecutor, Sche private TaskExecutorAdapter adaptedExecutor; + @Nullable + private TaskDecorator taskDecorator; + /** * Create a new ConcurrentTaskExecutor, using a single thread executor as default. @@ -139,6 +142,7 @@ public class ConcurrentTaskExecutor implements AsyncListenableTaskExecutor, Sche * @since 4.3 */ public final void setTaskDecorator(TaskDecorator taskDecorator) { + this.taskDecorator = taskDecorator; this.adaptedExecutor.setTaskDecorator(taskDecorator); } @@ -175,11 +179,15 @@ public class ConcurrentTaskExecutor implements AsyncListenableTaskExecutor, Sche } - 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; } diff --git a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskScheduler.java b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskScheduler.java index eddc2f58a9a..32e4592b99c 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskScheduler.java +++ b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskScheduler.java @@ -103,7 +103,7 @@ public class ConcurrentTaskScheduler extends ConcurrentTaskExecutor implements T */ public ConcurrentTaskScheduler() { super(); - this.scheduledExecutor = initScheduledExecutor(null); + initScheduledExecutor(null); } /** @@ -118,7 +118,7 @@ public class ConcurrentTaskScheduler extends ConcurrentTaskExecutor implements T */ public ConcurrentTaskScheduler(ScheduledExecutorService scheduledExecutor) { super(scheduledExecutor); - this.scheduledExecutor = initScheduledExecutor(scheduledExecutor); + initScheduledExecutor(scheduledExecutor); } /** @@ -134,11 +134,11 @@ public class ConcurrentTaskScheduler extends ConcurrentTaskExecutor implements T */ public ConcurrentTaskScheduler(Executor concurrentExecutor, ScheduledExecutorService scheduledExecutor) { super(concurrentExecutor); - this.scheduledExecutor = initScheduledExecutor(scheduledExecutor); + initScheduledExecutor(scheduledExecutor); } - private ScheduledExecutorService initScheduledExecutor(@Nullable ScheduledExecutorService scheduledExecutor) { + private void initScheduledExecutor(@Nullable ScheduledExecutorService scheduledExecutor) { if (scheduledExecutor != null) { this.scheduledExecutor = scheduledExecutor; this.enterpriseConcurrentScheduler = (managedScheduledExecutorServiceClass != null && @@ -148,7 +148,6 @@ public class ConcurrentTaskScheduler extends ConcurrentTaskExecutor implements T this.scheduledExecutor = Executors.newSingleThreadScheduledExecutor(); this.enterpriseConcurrentScheduler = false; } - return this.scheduledExecutor; } /** diff --git a/spring-context/src/test/java/org/springframework/scheduling/concurrent/ConcurrentTaskExecutorTests.java b/spring-context/src/test/java/org/springframework/scheduling/concurrent/ConcurrentTaskExecutorTests.java index 70830ce0494..bc8ac2ab55a 100644 --- a/spring-context/src/test/java/org/springframework/scheduling/concurrent/ConcurrentTaskExecutorTests.java +++ b/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.AfterEach; 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); @@ -69,10 +72,40 @@ class ConcurrentTaskExecutorTests extends AbstractSchedulingTaskExecutorTests { } @Test - void passingNullExecutorToSetterResultsInDefaultTaskExecutorBeingUsed() { + void earlySetConcurrentExecutorCallRespectsConfiguredTaskDecorator() { ConcurrentTaskExecutor executor = new ConcurrentTaskExecutor(); - executor.setConcurrentExecutor(null); + executor.setConcurrentExecutor(new DecoratedExecutor()); + executor.setTaskDecorator(new RunnableDecorator()); assertThatCode(() -> executor.execute(new NoOpRunnable())).doesNotThrowAnyException(); } + @Test + void lateSetConcurrentExecutorCallRespectsConfiguredTaskDecorator() { + ConcurrentTaskExecutor executor = new ConcurrentTaskExecutor(); + 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"); + } + } + }