root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions Packages Projects Releases Wiki Activity

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
This commit is contained in:
Stéphane Nicoll 2023-12-08 15:06:55 +01:00
parent 2eba3510f7
commit b78aed99ea
3 changed files with 115 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -89,6 +89,7 @@ public class ConcurrentTaskScheduler extends ConcurrentTaskExecutor implements T
}
@Nullable
private ScheduledExecutorService scheduledExecutor;
private boolean enterpriseConcurrentScheduler = false;
@ -168,6 +169,13 @@ public class ConcurrentTaskScheduler extends ConcurrentTaskExecutor implements T
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,6 +203,7 @@ public class ConcurrentTaskScheduler extends ConcurrentTaskExecutor implements T
@Override
@Nullable
public ScheduledFuture<?> schedule(Runnable task, Trigger trigger) {
ScheduledExecutorService scheduleExecutorToUse = getScheduledExecutor();
try {
if (this.enterpriseConcurrentScheduler) {
return new EnterpriseConcurrentTriggerScheduler().schedule(decorateTask(task, true), trigger);
@ -202,68 +211,73 @@ public class ConcurrentTaskScheduler extends ConcurrentTaskExecutor implements T
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 @@ public class ConcurrentTaskScheduler extends ConcurrentTaskExecutor implements T
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));
}

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

@ -28,6 +28,8 @@ import org.springframework.jndi.JndiTemplate;
/**
* 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.

86
spring-context/src/test/java/org/springframework/scheduling/concurrent/DefaultManagedTaskSchedulerTests.java Normal file
View File

@ -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");
}
}