Skip to content

Commit

Permalink
Deprecate default constructor on ConcurrentTaskExecutor/Scheduler
Browse files Browse the repository at this point in the history
Includes revision of null Executor configuration.
Respects TaskDecorator configuration on DefaultManagedTaskExecutor.

Closes spring-projectsgh-27914
  • Loading branch information
jhoeller authored and mdeinum committed Jun 29, 2023
1 parent ce84ef3 commit 7d025c6
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

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


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

Expand All @@ -139,6 +151,7 @@ public final Executor getConcurrentExecutor() {
* @since 4.3
*/
public final void setTaskDecorator(TaskDecorator taskDecorator) {
this.taskDecorator = taskDecorator;
this.adaptedExecutor.setTaskDecorator(taskDecorator);
}

Expand Down Expand Up @@ -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;
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand All @@ -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);
}
}

/**
Expand All @@ -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));
}

/**
Expand All @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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));
}

}
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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);
}

}
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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;
Expand All @@ -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;

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

}

0 comments on commit 7d025c6

Please sign in to comment.