Polishing

This commit is contained in:
Juergen Hoeller 2023-06-07 17:11:46 +02:00
parent 06bb6dbcff
commit b3597f3d99
4 changed files with 50 additions and 41 deletions

View File

@ -664,7 +664,7 @@ public class ScheduledAnnotationBeanPostProcessor
@Override @Override
public boolean requiresDestruction(Object bean) { public boolean requiresDestruction(Object bean) {
synchronized (this.scheduledTasks) { synchronized (this.scheduledTasks) {
return this.scheduledTasks.containsKey(bean) || this.reactiveSubscriptions.containsKey(bean); return (this.scheduledTasks.containsKey(bean) || this.reactiveSubscriptions.containsKey(bean));
} }
} }

View File

@ -44,7 +44,7 @@ import org.springframework.util.StringUtils;
* cases without a dependency on optional classes. * cases without a dependency on optional classes.
* *
* @author Simon Baslé * @author Simon Baslé
* @since 6.1.0 * @since 6.1
*/ */
abstract class ScheduledAnnotationReactiveSupport { abstract class ScheduledAnnotationReactiveSupport {
@ -54,7 +54,8 @@ abstract class ScheduledAnnotationReactiveSupport {
static final boolean coroutinesReactorPresent = ClassUtils.isPresent( static final boolean coroutinesReactorPresent = ClassUtils.isPresent(
"kotlinx.coroutines.reactor.MonoKt", ScheduledAnnotationReactiveSupport.class.getClassLoader()); "kotlinx.coroutines.reactor.MonoKt", ScheduledAnnotationReactiveSupport.class.getClassLoader());
private static final Log LOGGER = LogFactory.getLog(ScheduledAnnotationReactiveSupport.class); private static final Log logger = LogFactory.getLog(ScheduledAnnotationReactiveSupport.class);
/** /**
* Checks that if the method is reactive, it can be scheduled. Methods are considered * Checks that if the method is reactive, it can be scheduled. Methods are considered
@ -72,10 +73,10 @@ abstract class ScheduledAnnotationReactiveSupport {
if (KotlinDetector.isKotlinPresent() && KotlinDetector.isSuspendingFunction(method)) { if (KotlinDetector.isKotlinPresent() && KotlinDetector.isSuspendingFunction(method)) {
// Note that suspending functions declared without args have a single Continuation // Note that suspending functions declared without args have a single Continuation
// parameter in reflective inspection // parameter in reflective inspection
Assert.isTrue(method.getParameterCount() == 1,"Kotlin suspending functions may only be " Assert.isTrue(method.getParameterCount() == 1,
+ "annotated with @Scheduled if declared without arguments"); "Kotlin suspending functions may only be annotated with @Scheduled if declared without arguments");
Assert.isTrue(coroutinesReactorPresent, "Kotlin suspending functions may only be annotated with " Assert.isTrue(coroutinesReactorPresent, "Kotlin suspending functions may only be annotated with " +
+ "@Scheduled if the Coroutine-Reactor bridge (kotlinx.coroutines.reactor) is present at runtime"); "@Scheduled if the Coroutine-Reactor bridge (kotlinx.coroutines.reactor) is present at runtime");
return true; return true;
} }
ReactiveAdapterRegistry registry = ReactiveAdapterRegistry.getSharedInstance(); ReactiveAdapterRegistry registry = ReactiveAdapterRegistry.getSharedInstance();
@ -87,10 +88,10 @@ abstract class ScheduledAnnotationReactiveSupport {
if (candidateAdapter == null) { if (candidateAdapter == null) {
return false; return false;
} }
Assert.isTrue(method.getParameterCount() == 0, "Reactive methods may only be annotated with " Assert.isTrue(method.getParameterCount() == 0,
+ "@Scheduled if declared without arguments"); "Reactive methods may only be annotated with @Scheduled if declared without arguments");
Assert.isTrue(candidateAdapter.getDescriptor().isDeferred(), "Reactive methods may only be annotated with " Assert.isTrue(candidateAdapter.getDescriptor().isDeferred(),
+ "@Scheduled if the return type supports deferred execution"); "Reactive methods may only be annotated with @Scheduled if the return type supports deferred execution");
return true; return true;
} }
@ -112,23 +113,23 @@ abstract class ScheduledAnnotationReactiveSupport {
Class<?> returnType = method.getReturnType(); Class<?> returnType = method.getReturnType();
ReactiveAdapter adapter = registry.getAdapter(returnType); ReactiveAdapter adapter = registry.getAdapter(returnType);
if (adapter == null) { if (adapter == null) {
throw new IllegalArgumentException("Cannot convert the @Scheduled reactive method return type to Publisher"); throw new IllegalArgumentException("Cannot convert @Scheduled reactive method return type to Publisher");
} }
if (!adapter.getDescriptor().isDeferred()) { if (!adapter.getDescriptor().isDeferred()) {
throw new IllegalArgumentException("Cannot convert the @Scheduled reactive method return type to Publisher: " throw new IllegalArgumentException("Cannot convert @Scheduled reactive method return type to Publisher: " +
+ returnType.getSimpleName() + " is not a deferred reactive type"); returnType.getSimpleName() + " is not a deferred reactive type");
} }
Method invocableMethod = AopUtils.selectInvocableMethod(method, bean.getClass()); Method invocableMethod = AopUtils.selectInvocableMethod(method, bean.getClass());
try { try {
ReflectionUtils.makeAccessible(invocableMethod); ReflectionUtils.makeAccessible(invocableMethod);
Object r = invocableMethod.invoke(bean); Object returnValue = invocableMethod.invoke(bean);
Publisher<?> publisher = adapter.toPublisher(r); Publisher<?> publisher = adapter.toPublisher(returnValue);
// If Reactor is on the classpath, we could benefit from having a checkpoint for debuggability // If Reactor is on the classpath, we could benefit from having a checkpoint for debuggability
if (reactorPresent) { if (reactorPresent) {
final String checkpoint = "@Scheduled '"+ method.getName() + "()' in bean '" return Flux.from(publisher).checkpoint(
+ method.getDeclaringClass().getName() + "'"; "@Scheduled '"+ method.getName() + "()' in '" + method.getDeclaringClass().getName() + "'");
return Flux.from(publisher).checkpoint(checkpoint);
} }
else { else {
return publisher; return publisher;
@ -157,20 +158,24 @@ abstract class ScheduledAnnotationReactiveSupport {
*/ */
static Runnable createSubscriptionRunnable(Method method, Object targetBean, Scheduled scheduled, static Runnable createSubscriptionRunnable(Method method, Object targetBean, Scheduled scheduled,
List<Runnable> subscriptionTrackerRegistry) { List<Runnable> subscriptionTrackerRegistry) {
boolean shouldBlock = scheduled.fixedDelay() > 0 || StringUtils.hasText(scheduled.fixedDelayString());
final Publisher<?> publisher = getPublisherFor(method, targetBean); boolean shouldBlock = (scheduled.fixedDelay() > 0 || StringUtils.hasText(scheduled.fixedDelayString()));
Publisher<?> publisher = getPublisherFor(method, targetBean);
return new SubscribingRunnable(publisher, shouldBlock, subscriptionTrackerRegistry); return new SubscribingRunnable(publisher, shouldBlock, subscriptionTrackerRegistry);
} }
/** /**
* Utility implementation of {@code Runnable} that subscribes to a {@code Publisher} * Utility implementation of {@code Runnable} that subscribes to a {@code Publisher}
* or subscribes-then-blocks if {@code shouldBlock} is set to {@code true}. * or subscribes-then-blocks if {@code shouldBlock} is set to {@code true}.
*/ */
static final class SubscribingRunnable implements Runnable { static final class SubscribingRunnable implements Runnable {
final Publisher<?> publisher; private final Publisher<?> publisher;
final boolean shouldBlock; final boolean shouldBlock;
final List<Runnable> subscriptionTrackerRegistry;
private final List<Runnable> subscriptionTrackerRegistry;
SubscribingRunnable(Publisher<?> publisher, boolean shouldBlock, List<Runnable> subscriptionTrackerRegistry) { SubscribingRunnable(Publisher<?> publisher, boolean shouldBlock, List<Runnable> subscriptionTrackerRegistry) {
this.publisher = publisher; this.publisher = publisher;
@ -181,7 +186,7 @@ abstract class ScheduledAnnotationReactiveSupport {
@Override @Override
public void run() { public void run() {
if (this.shouldBlock) { if (this.shouldBlock) {
final CountDownLatch latch = new CountDownLatch(1); CountDownLatch latch = new CountDownLatch(1);
TrackingSubscriber subscriber = new TrackingSubscriber(this.subscriptionTrackerRegistry, latch); TrackingSubscriber subscriber = new TrackingSubscriber(this.subscriptionTrackerRegistry, latch);
this.subscriptionTrackerRegistry.add(subscriber); this.subscriptionTrackerRegistry.add(subscriber);
this.publisher.subscribe(subscriber); this.publisher.subscribe(subscriber);
@ -193,13 +198,14 @@ abstract class ScheduledAnnotationReactiveSupport {
} }
} }
else { else {
final TrackingSubscriber subscriber = new TrackingSubscriber(this.subscriptionTrackerRegistry); TrackingSubscriber subscriber = new TrackingSubscriber(this.subscriptionTrackerRegistry);
this.subscriptionTrackerRegistry.add(subscriber); this.subscriptionTrackerRegistry.add(subscriber);
this.publisher.subscribe(subscriber); this.publisher.subscribe(subscriber);
} }
} }
} }
/** /**
* A {@code Subscriber} which keeps track of its {@code Subscription} and exposes the * A {@code Subscriber} which keeps track of its {@code Subscription} and exposes the
* capacity to cancel the subscription as a {@code Runnable}. Can optionally support * capacity to cancel the subscription as a {@code Runnable}. Can optionally support
@ -246,13 +252,13 @@ abstract class ScheduledAnnotationReactiveSupport {
@Override @Override
public void onNext(Object obj) { public void onNext(Object obj) {
// NO-OP // no-op
} }
@Override @Override
public void onError(Throwable ex) { public void onError(Throwable ex) {
this.subscriptionTrackerRegistry.remove(this); this.subscriptionTrackerRegistry.remove(this);
LOGGER.warn("Unexpected error occurred in scheduled reactive task", ex); logger.warn("Unexpected error occurred in scheduled reactive task", ex);
if (this.blockingLatch != null) { if (this.blockingLatch != null) {
this.blockingLatch.countDown(); this.blockingLatch.countDown();
} }

View File

@ -41,6 +41,10 @@ import static org.springframework.scheduling.annotation.ScheduledAnnotationReact
import static org.springframework.scheduling.annotation.ScheduledAnnotationReactiveSupport.getPublisherFor; import static org.springframework.scheduling.annotation.ScheduledAnnotationReactiveSupport.getPublisherFor;
import static org.springframework.scheduling.annotation.ScheduledAnnotationReactiveSupport.isReactive; import static org.springframework.scheduling.annotation.ScheduledAnnotationReactiveSupport.isReactive;
/**
* @author Simon Baslé
* @since 6.1
*/
class ScheduledAnnotationReactiveSupportTests { class ScheduledAnnotationReactiveSupportTests {
@Test @Test
@ -165,9 +169,10 @@ class ScheduledAnnotationReactiveSupportTests {
.as("checkpoint class") .as("checkpoint class")
.isEqualTo("reactor.core.publisher.FluxOnAssembly"); .isEqualTo("reactor.core.publisher.FluxOnAssembly");
assertThat(p).hasToString("checkpoint(\"@Scheduled 'mono()' in bean 'org.springframework.scheduling.annotation.ScheduledAnnotationReactiveSupportTests$ReactiveMethods'\")"); assertThat(p).hasToString("checkpoint(\"@Scheduled 'mono()' in 'org.springframework.scheduling.annotation.ScheduledAnnotationReactiveSupportTests$ReactiveMethods'\")");
} }
static class ReactiveMethods { static class ReactiveMethods {
public String oops() { public String oops() {
@ -226,14 +231,12 @@ class ScheduledAnnotationReactiveSupportTests {
AtomicInteger subscription = new AtomicInteger(); AtomicInteger subscription = new AtomicInteger();
public Mono<Void> trackingMono() { public Mono<Void> trackingMono() {
return Mono.<Void>empty() return Mono.<Void>empty().doOnSubscribe(s -> subscription.incrementAndGet());
.doOnSubscribe(s -> subscription.incrementAndGet());
} }
public Mono<Void> monoError() { public Mono<Void> monoError() {
return Mono.error(new IllegalStateException("expected")); return Mono.error(new IllegalStateException("expected"));
} }
} }
} }

View File

@ -92,7 +92,7 @@ class KotlinScheduledAnnotationReactiveSupportTests {
//static helper method //static helper method
Assertions.assertThatIllegalArgumentException().isThrownBy { getPublisherFor(m!!, target!!) } Assertions.assertThatIllegalArgumentException().isThrownBy { getPublisherFor(m!!, target!!) }
.withMessage("Cannot convert the @Scheduled reactive method return type to Publisher") .withMessage("Cannot convert @Scheduled reactive method return type to Publisher")
.withNoCause() .withNoCause()
} }