From 38bd4bd58cbd5c34c64ca4db65e8797db4f0fb86 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Thu, 12 Apr 2018 15:41:15 +0200 Subject: [PATCH] Stop associating an Executor bean to Quartz This commits make sure that the Quartz auto-configuration no longer associates an `Executor` bean if present in the context as Quartz offers properties to tune it, which would mutate and lead to unexpected results. Closes gh-12823 --- .../quartz/QuartzAutoConfiguration.java | 9 +- .../quartz/QuartzAutoConfigurationTests.java | 99 ++----------------- .../main/asciidoc/spring-boot-features.adoc | 4 + 3 files changed, 15 insertions(+), 97 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/quartz/QuartzAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/quartz/QuartzAutoConfiguration.java index 76936be476..dbc5475407 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/quartz/QuartzAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/quartz/QuartzAutoConfiguration.java @@ -19,7 +19,6 @@ package org.springframework.boot.autoconfigure.quartz; import java.util.List; import java.util.Map; import java.util.Properties; -import java.util.concurrent.Executor; import javax.sql.DataSource; @@ -64,8 +63,6 @@ public class QuartzAutoConfiguration { private final List customizers; - private final Executor taskExecutor; - private final JobDetail[] jobDetails; private final Map calendars; @@ -76,12 +73,11 @@ public class QuartzAutoConfiguration { public QuartzAutoConfiguration(QuartzProperties properties, ObjectProvider> customizers, - ObjectProvider taskExecutor, ObjectProvider jobDetails, + ObjectProvider jobDetails, ObjectProvider> calendars, ObjectProvider triggers, ApplicationContext applicationContext) { this.properties = properties; this.customizers = customizers.getIfAvailable(); - this.taskExecutor = taskExecutor.getIfUnique(); this.jobDetails = jobDetails.getIfAvailable(); this.calendars = calendars.getIfAvailable(); this.triggers = triggers.getIfAvailable(); @@ -98,9 +94,6 @@ public class QuartzAutoConfiguration { schedulerFactoryBean .setQuartzProperties(asProperties(this.properties.getProperties())); } - if (this.taskExecutor != null) { - schedulerFactoryBean.setTaskExecutor(this.taskExecutor); - } if (this.jobDetails != null && this.jobDetails.length > 0) { schedulerFactoryBean.setJobDetails(this.jobDetails); } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/quartz/QuartzAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/quartz/QuartzAutoConfigurationTests.java index c1eaf917dc..eb6e4ce21e 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/quartz/QuartzAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/quartz/QuartzAutoConfigurationTests.java @@ -17,7 +17,6 @@ package org.springframework.boot.autoconfigure.quartz; import java.util.concurrent.Executor; -import java.util.concurrent.Executors; import javax.sql.DataSource; @@ -36,7 +35,6 @@ import org.quartz.TriggerKey; import org.quartz.impl.calendar.MonthlyCalendar; import org.quartz.impl.calendar.WeeklyCalendar; import org.quartz.simpl.RAMJobStore; -import org.quartz.simpl.SimpleThreadPool; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.AutoConfigurations; @@ -54,12 +52,13 @@ import org.springframework.context.annotation.Primary; import org.springframework.core.env.Environment; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.scheduling.quartz.LocalDataSourceJobStore; -import org.springframework.scheduling.quartz.LocalTaskExecutorThreadPool; import org.springframework.scheduling.quartz.QuartzJobBean; import org.springframework.util.Assert; import static org.assertj.core.api.Assertions.assertThat; import static org.hamcrest.CoreMatchers.containsString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyZeroInteractions; /** * Tests for {@link QuartzAutoConfiguration}. @@ -144,53 +143,15 @@ public class QuartzAutoConfigurationTests { @Test public void withTaskExecutor() { - this.contextRunner.withUserConfiguration(QuartzExecutorConfiguration.class) + this.contextRunner.withUserConfiguration(MockExecutorConfiguration.class) + .withPropertyValues( + "spring.quartz.properties.org.quartz.threadPool.threadCount=50") .run((context) -> { assertThat(context).hasSingleBean(Scheduler.class); Scheduler scheduler = context.getBean(Scheduler.class); - assertThat(scheduler.getMetaData().getThreadPoolClass()) - .isEqualTo(LocalTaskExecutorThreadPool.class); - }); - } - - @Test - public void withMultipleTaskExecutors() { - this.contextRunner - .withUserConfiguration(QuartzMultipleExecutorsConfiguration.class) - .run((context) -> { - assertThat(context.getBeansOfType(Executor.class)).hasSize(2); - assertThat(context).hasSingleBean(Scheduler.class); - Scheduler scheduler = context.getBean(Scheduler.class); - assertThat(scheduler.getMetaData().getThreadPoolClass()) - .isEqualTo(SimpleThreadPool.class); - }); - } - - @Test - public void withMultipleTaskExecutorsWithPrimary() { - this.contextRunner - .withUserConfiguration( - QuartzMultipleExecutorsWithPrimaryConfiguration.class) - .run((context) -> { - assertThat(context.getBeansOfType(Executor.class)).hasSize(2); - assertThat(context).hasSingleBean(Scheduler.class); - Scheduler scheduler = context.getBean(Scheduler.class); - assertThat(scheduler.getMetaData().getThreadPoolClass()) - .isEqualTo(LocalTaskExecutorThreadPool.class); - }); - } - - @Test - public void withMultipleTaskExecutorsWithCustomizer() { - this.contextRunner - .withUserConfiguration( - QuartzMultipleExecutorsWithCustomizerConfiguration.class) - .run((context) -> { - assertThat(context.getBeansOfType(Executor.class)).hasSize(3); - assertThat(context).hasSingleBean(Scheduler.class); - Scheduler scheduler = context.getBean(Scheduler.class); - assertThat(scheduler.getMetaData().getThreadPoolClass()) - .isEqualTo(LocalTaskExecutorThreadPool.class); + assertThat(scheduler.getMetaData().getThreadPoolSize()).isEqualTo(50); + Executor executor = context.getBean(Executor.class); + verifyZeroInteractions(executor); }); } @@ -304,51 +265,11 @@ public class QuartzAutoConfigurationTests { } @Configuration - protected static class QuartzExecutorConfiguration extends BaseQuartzConfiguration { + protected static class MockExecutorConfiguration extends BaseQuartzConfiguration { @Bean public Executor executor() { - return Executors.newSingleThreadExecutor(); - } - - } - - @Configuration - protected static class QuartzMultipleExecutorsConfiguration - extends QuartzExecutorConfiguration { - - @Bean - public Executor anotherExecutor() { - return Executors.newSingleThreadExecutor(); - } - - } - - @Configuration - protected static class QuartzMultipleExecutorsWithPrimaryConfiguration - extends QuartzExecutorConfiguration { - - @Bean - @Primary - public Executor primaryExecutor() { - return Executors.newSingleThreadExecutor(); - } - - } - - @Configuration - protected static class QuartzMultipleExecutorsWithCustomizerConfiguration - extends QuartzMultipleExecutorsConfiguration { - - @Bean - public Executor yetAnotherExecutor() { - return Executors.newSingleThreadExecutor(); - } - - @Bean - public SchedulerFactoryBeanCustomizer customizer() { - return (schedulerFactoryBean) -> schedulerFactoryBean - .setTaskExecutor(yetAnotherExecutor()); + return mock(Executor.class); } } diff --git a/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc b/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc index 604a3ee2f1..77e5015391 100644 --- a/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc +++ b/spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc @@ -5876,6 +5876,10 @@ Quartz Scheduler configuration can be customized by using Quartz configuration p ()`spring.quartz.properties.*`) and `SchedulerFactoryBeanCustomizer` beans, which allow programmatic `SchedulerFactoryBean` customization. +NOTE: In particular, an `Executor` bean is not associated with the scheduler as Quartz +offers a way to configure the scheduler via `spring.quartz.properties`. If you need +to customize the task executor, consider implementing `SchedulerFactoryBeanCustomizer`. + Jobs can define setters to inject data map properties. Regular beans can also be injected in a similar manner, as shown in the following example: