Polish async method execution infrastructure

In anticipation of substantive changes required to implement @Async
executor qualification, the following updates have been made to the
components and infrastructure supporting @Async functionality:

 - Fix trailing whitespace and indentation errors
 - Fix generics warnings
 - Add Javadoc where missing, update to use {@code} tags, etc.
 - Avoid NPE in AopUtils#canApply
 - Organize imports to follow conventions
 - Remove System.out.println statements from tests
 - Correct various punctuation and grammar problems
This commit is contained in:
Chris Beams 2012-05-19 11:04:36 +03:00
parent 096693c46f
commit 3fb11870d9
10 changed files with 110 additions and 92 deletions

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2009 the original author or authors. * Copyright 2002-2012 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -55,14 +55,16 @@ public class AsyncExecutionInterceptor implements MethodInterceptor, Ordered {
/** /**
* Create a new AsyncExecutionInterceptor. * Create a new {@code AsyncExecutionInterceptor}.
* @param asyncExecutor the Spring AsyncTaskExecutor to delegate to * @param executor the {@link Executor} (typically a Spring {@link AsyncTaskExecutor}
* or {@link java.util.concurrent.ExecutorService}) to delegate to.
*/ */
public AsyncExecutionInterceptor(AsyncTaskExecutor asyncExecutor) { public AsyncExecutionInterceptor(AsyncTaskExecutor asyncExecutor) {
Assert.notNull(asyncExecutor, "TaskExecutor must not be null"); Assert.notNull(asyncExecutor, "TaskExecutor must not be null");
this.asyncExecutor = asyncExecutor; this.asyncExecutor = asyncExecutor;
} }
/** /**
* Create a new AsyncExecutionInterceptor. * Create a new AsyncExecutionInterceptor.
* @param asyncExecutor the <code>java.util.concurrent</code> Executor * @param asyncExecutor the <code>java.util.concurrent</code> Executor
@ -74,20 +76,21 @@ public class AsyncExecutionInterceptor implements MethodInterceptor, Ordered {
public Object invoke(final MethodInvocation invocation) throws Throwable { public Object invoke(final MethodInvocation invocation) throws Throwable {
Future result = this.asyncExecutor.submit(new Callable<Object>() { Future<?> result = this.asyncExecutor.submit(
public Object call() throws Exception { new Callable<Object>() {
try { public Object call() throws Exception {
Object result = invocation.proceed(); try {
if (result instanceof Future) { Object result = invocation.proceed();
return ((Future) result).get(); if (result instanceof Future) {
return ((Future<?>) result).get();
}
}
catch (Throwable ex) {
ReflectionUtils.rethrowException(ex);
}
return null;
} }
} });
catch (Throwable ex) {
ReflectionUtils.rethrowException(ex);
}
return null;
}
});
if (Future.class.isAssignableFrom(invocation.getMethod().getReturnType())) { if (Future.class.isAssignableFrom(invocation.getMethod().getReturnType())) {
return result; return result;
} }

View File

@ -206,6 +206,7 @@ public abstract class AopUtils {
* @return whether the pointcut can apply on any method * @return whether the pointcut can apply on any method
*/ */
public static boolean canApply(Pointcut pc, Class<?> targetClass, boolean hasIntroductions) { public static boolean canApply(Pointcut pc, Class<?> targetClass, boolean hasIntroductions) {
Assert.notNull(pc, "Pointcut must not be null");
if (!pc.getClassFilter().matches(targetClass)) { if (!pc.getClassFilter().matches(targetClass)) {
return false; return false;
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2010 the original author or authors. * Copyright 2002-2012 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -49,10 +49,17 @@ public abstract aspect AbstractAsyncExecutionAspect {
} }
} }
/**
* Apply around advice to methods matching the {@link #asyncMethod()} pointcut,
* submit the actual calling of the method to the correct task executor and return
* immediately to the caller.
* @return {@link Future} if the original method returns {@code Future}; {@code null}
* otherwise.
*/
Object around() : asyncMethod() { Object around() : asyncMethod() {
if (this.asyncExecutor == null) { if (this.asyncExecutor == null) {
return proceed(); return proceed();
} }
Callable<Object> callable = new Callable<Object>() { Callable<Object> callable = new Callable<Object>() {
public Object call() throws Exception { public Object call() throws Exception {
Object result = proceed(); Object result = proceed();
@ -70,6 +77,9 @@ public abstract aspect AbstractAsyncExecutionAspect {
} }
} }
/**
* Return the set of joinpoints at which async advice should be applied.
*/
public abstract pointcut asyncMethod(); public abstract pointcut asyncMethod();
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2010 the original author or authors. * Copyright 2002-2012 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -24,11 +24,11 @@ import org.springframework.scheduling.annotation.Async;
* *
* <p>This aspect routes methods marked with the {@link Async} annotation * <p>This aspect routes methods marked with the {@link Async} annotation
* as well as methods in classes marked with the same. Any method expected * as well as methods in classes marked with the same. Any method expected
* to be routed asynchronously must return either void, {@link Future}, * to be routed asynchronously must return either {@code void}, {@link Future},
* or a subtype of {@link Future}. This aspect, therefore, will produce * or a subtype of {@link Future}. This aspect, therefore, will produce
* a compile-time error for methods that violate this constraint on the return type. * a compile-time error for methods that violate this constraint on the return type.
* If, however, a class marked with <code>&#64;Async</code> contains a method that * If, however, a class marked with {@code @Async} contains a method that violates this
* violates this constraint, it produces only a warning. * constraint, it produces only a warning.
* *
* @author Ramnivas Laddad * @author Ramnivas Laddad
* @since 3.0.5 * @since 3.0.5
@ -49,6 +49,7 @@ public aspect AnnotationAsyncExecutionAspect extends AbstractAsyncExecutionAspec
declare warning: declare warning:
execution(!(void||Future) (@Async *).*(..)): execution(!(void||Future) (@Async *).*(..)):
"Methods in a class marked with @Async that do not return void or Future will be routed synchronously"; "Methods in a class marked with @Async that do not return void or Future will " +
"be routed synchronously";
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2010 the original author or authors. * Copyright 2002-2012 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -20,22 +20,22 @@ import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future; import java.util.concurrent.Future;
import junit.framework.Assert;
import static junit.framework.Assert.*;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import org.springframework.core.task.SimpleAsyncTaskExecutor; import org.springframework.core.task.SimpleAsyncTaskExecutor;
import org.springframework.scheduling.annotation.Async; import org.springframework.scheduling.annotation.Async;
import org.springframework.scheduling.annotation.AsyncResult; import org.springframework.scheduling.annotation.AsyncResult;
import static org.junit.Assert.*;
/** /**
* Unit tests for {@link AnnotationAsyncExecutionAspect}.
*
* @author Ramnivas Laddad * @author Ramnivas Laddad
*/ */
public class AnnotationAsyncExecutionAspectTests { public class AnnotationAsyncExecutionAspectTests {
private static final long WAIT_TIME = 1000; //milli seconds private static final long WAIT_TIME = 1000; //milliseconds
private CountingExecutor executor; private CountingExecutor executor;
@ -104,6 +104,7 @@ public class AnnotationAsyncExecutionAspectTests {
assertEquals(0, executor.submitCompleteCounter); assertEquals(0, executor.submitCompleteCounter);
} }
@SuppressWarnings("serial") @SuppressWarnings("serial")
private static class CountingExecutor extends SimpleAsyncTaskExecutor { private static class CountingExecutor extends SimpleAsyncTaskExecutor {
int submitStartCounter; int submitStartCounter;
@ -124,11 +125,12 @@ public class AnnotationAsyncExecutionAspectTests {
try { try {
wait(WAIT_TIME); wait(WAIT_TIME);
} catch (InterruptedException e) { } catch (InterruptedException e) {
Assert.fail("Didn't finish the async job in " + WAIT_TIME + " milliseconds"); fail("Didn't finish the async job in " + WAIT_TIME + " milliseconds");
} }
} }
} }
static class ClassWithoutAsyncAnnotation { static class ClassWithoutAsyncAnnotation {
int counter; int counter;
@ -145,17 +147,19 @@ public class AnnotationAsyncExecutionAspectTests {
return new AsyncResult<Integer>(5); return new AsyncResult<Integer>(5);
} }
// It should be an error to attach @Async to a method that returns a non-void /**
// or non-Future. * It should raise an error to attach @Async to a method that returns a non-void
// We need to keep this commented out, otherwise there will be a compile-time error. * or non-Future. This method must remain commented-out, otherwise there will be a
// Please uncomment and re-comment this periodically to check that the compiler * compile-time error. Uncomment to manually verify that the compiler produces an
// produces an error message due to the 'declare error' statement * error message due to the 'declare error' statement in
// in AnnotationAsyncExecutionAspect * {@link AnnotationAsyncExecutionAspect}.
*/
// @Async public int getInt() { // @Async public int getInt() {
// return 0; // return 0;
// } // }
} }
@Async @Async
static class ClassWithAsyncAnnotation { static class ClassWithAsyncAnnotation {
int counter; int counter;
@ -164,7 +168,8 @@ public class AnnotationAsyncExecutionAspectTests {
counter++; counter++;
} }
// Manually check that there is a warning from the 'declare warning' statement in AnnotationAsynchExecutionAspect // Manually check that there is a warning from the 'declare warning' statement in
// AnnotationAsyncExecutionAspect
public int return5() { public int return5() {
return 5; return 5;
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2009 the original author or authors. * Copyright 2002-2012 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -28,13 +28,13 @@ import java.lang.annotation.Target;
* considered as asynchronous. * considered as asynchronous.
* *
* <p>In terms of target method signatures, any parameter types are supported. * <p>In terms of target method signatures, any parameter types are supported.
* However, the return type is constrained to either <code>void</code> or * However, the return type is constrained to either {@code void} or
* <code>java.util.concurrent.Future</code>. In the latter case, the Future handle * {@link java.util.concurrent.Future}. In the latter case, the {@code Future} handle
* returned from the proxy will be an actual asynchronous Future that can be used * returned from the proxy will be an actual asynchronous {@code Future} that can be used
* to track the result of the asynchronous method execution. However, since the * to track the result of the asynchronous method execution. However, since the
* target method needs to implement the same signature, it will have to return * target method needs to implement the same signature, it will have to return
* a temporary Future handle that just passes the return value through: e.g. * a temporary {@code Future} handle that just passes the return value through: e.g.
* Spring's {@link AsyncResult} or EJB 3.1's <code>javax.ejb.AsyncResult</code>. * Spring's {@link AsyncResult} or EJB 3.1's {@link javax.ejb.AsyncResult}.
* *
* @author Juergen Hoeller * @author Juergen Hoeller
* @since 3.0 * @since 3.0

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2009 the original author or authors. * Copyright 2002-2012 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -50,6 +50,7 @@ import org.springframework.util.Assert;
* @see org.springframework.dao.DataAccessException * @see org.springframework.dao.DataAccessException
* @see org.springframework.dao.support.PersistenceExceptionTranslator * @see org.springframework.dao.support.PersistenceExceptionTranslator
*/ */
@SuppressWarnings("serial")
public class AsyncAnnotationAdvisor extends AbstractPointcutAdvisor { public class AsyncAnnotationAdvisor extends AbstractPointcutAdvisor {
private Advice advice; private Advice advice;
@ -58,14 +59,14 @@ public class AsyncAnnotationAdvisor extends AbstractPointcutAdvisor {
/** /**
* Create a new ConcurrencyAnnotationBeanPostProcessor for bean-style configuration. * Create a new {@code AsyncAnnotationAdvisor} for bean-style configuration.
*/ */
public AsyncAnnotationAdvisor() { public AsyncAnnotationAdvisor() {
this(new SimpleAsyncTaskExecutor()); this(new SimpleAsyncTaskExecutor());
} }
/** /**
* Create a new ConcurrencyAnnotationBeanPostProcessor for the given task executor. * Create a new {@code AsyncAnnotationAdvisor} for the given task executor.
* @param executor the task executor to use for asynchronous methods * @param executor the task executor to use for asynchronous methods
*/ */
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
@ -74,7 +75,7 @@ public class AsyncAnnotationAdvisor extends AbstractPointcutAdvisor {
asyncAnnotationTypes.add(Async.class); asyncAnnotationTypes.add(Async.class);
ClassLoader cl = AsyncAnnotationAdvisor.class.getClassLoader(); ClassLoader cl = AsyncAnnotationAdvisor.class.getClassLoader();
try { try {
asyncAnnotationTypes.add((Class) cl.loadClass("javax.ejb.Asynchronous")); asyncAnnotationTypes.add((Class<? extends Annotation>) cl.loadClass("javax.ejb.Asynchronous"));
} }
catch (ClassNotFoundException ex) { catch (ClassNotFoundException ex) {
// If EJB 3.1 API not present, simply ignore. // If EJB 3.1 API not present, simply ignore.

View File

@ -35,7 +35,7 @@
<xsd:documentation><![CDATA[ <xsd:documentation><![CDATA[
Specifies the java.util.Executor instance to use when invoking asynchronous methods. Specifies the java.util.Executor instance to use when invoking asynchronous methods.
If not provided, an instance of org.springframework.core.task.SimpleAsyncTaskExecutor If not provided, an instance of org.springframework.core.task.SimpleAsyncTaskExecutor
will be used by default will be used by default.
]]></xsd:documentation> ]]></xsd:documentation>
</xsd:annotation> </xsd:annotation>
</xsd:attribute> </xsd:attribute>

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2009 the original author or authors. * Copyright 2002-2012 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -18,8 +18,6 @@ package org.springframework.scheduling.annotation;
import java.util.concurrent.Future; import java.util.concurrent.Future;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import org.junit.Test; import org.junit.Test;
import org.springframework.aop.framework.autoproxy.DefaultAdvisorAutoProxyCreator; import org.springframework.aop.framework.autoproxy.DefaultAdvisorAutoProxyCreator;
@ -27,7 +25,8 @@ import org.springframework.beans.factory.support.RootBeanDefinition;
import org.springframework.context.ApplicationEvent; import org.springframework.context.ApplicationEvent;
import org.springframework.context.ApplicationListener; import org.springframework.context.ApplicationListener;
import org.springframework.context.support.GenericApplicationContext; import org.springframework.context.support.GenericApplicationContext;
import org.springframework.scheduling.annotation.AsyncResult;
import static org.junit.Assert.*;
/** /**
* @author Juergen Hoeller * @author Juergen Hoeller
@ -155,7 +154,6 @@ public class AsyncExecutionTests {
@Async @Async
public void doSomething(int i) { public void doSomething(int i) {
System.out.println(Thread.currentThread().getName() + ": " + i);
assertTrue(!Thread.currentThread().getName().equals(originalThreadName)); assertTrue(!Thread.currentThread().getName().equals(originalThreadName));
} }
@ -171,7 +169,6 @@ public class AsyncExecutionTests {
public static class AsyncClassBean { public static class AsyncClassBean {
public void doSomething(int i) { public void doSomething(int i) {
System.out.println(Thread.currentThread().getName() + ": " + i);
assertTrue(!Thread.currentThread().getName().equals(originalThreadName)); assertTrue(!Thread.currentThread().getName().equals(originalThreadName));
} }
@ -194,7 +191,6 @@ public class AsyncExecutionTests {
public static class AsyncInterfaceBean implements AsyncInterface { public static class AsyncInterfaceBean implements AsyncInterface {
public void doSomething(int i) { public void doSomething(int i) {
System.out.println(Thread.currentThread().getName() + ": " + i);
assertTrue(!Thread.currentThread().getName().equals(originalThreadName)); assertTrue(!Thread.currentThread().getName().equals(originalThreadName));
} }
@ -224,7 +220,6 @@ public class AsyncExecutionTests {
} }
public void doSomething(int i) { public void doSomething(int i) {
System.out.println(Thread.currentThread().getName() + ": " + i);
assertTrue(!Thread.currentThread().getName().equals(originalThreadName)); assertTrue(!Thread.currentThread().getName().equals(originalThreadName));
} }
@ -235,7 +230,7 @@ public class AsyncExecutionTests {
} }
public static class AsyncMethodListener implements ApplicationListener { public static class AsyncMethodListener implements ApplicationListener<ApplicationEvent> {
@Async @Async
public void onApplicationEvent(ApplicationEvent event) { public void onApplicationEvent(ApplicationEvent event) {
@ -246,7 +241,7 @@ public class AsyncExecutionTests {
@Async @Async
public static class AsyncClassListener implements ApplicationListener { public static class AsyncClassListener implements ApplicationListener<ApplicationEvent> {
public AsyncClassListener() { public AsyncClassListener() {
listenerConstructed++; listenerConstructed++;

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2011 the original author or authors. * Copyright 2002-2012 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -16,18 +16,15 @@
package org.springframework.scheduling.annotation; package org.springframework.scheduling.annotation;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import java.lang.annotation.ElementType; import java.lang.annotation.ElementType;
import java.lang.annotation.Retention; import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy; import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target; import java.lang.annotation.Target;
import java.util.concurrent.Executor; import java.util.concurrent.Executor;
import org.junit.Test; import org.junit.Test;
import org.springframework.aop.Advisor; import org.springframework.aop.Advisor;
import org.springframework.aop.framework.Advised; import org.springframework.aop.framework.Advised;
import org.springframework.aop.support.AopUtils; import org.springframework.aop.support.AopUtils;
@ -39,6 +36,11 @@ import org.springframework.context.annotation.Configuration;
import org.springframework.core.Ordered; import org.springframework.core.Ordered;
import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor; import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor;
import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.Assert.*;
/** /**
* Tests use of @EnableAsync on @Configuration classes. * Tests use of @EnableAsync on @Configuration classes.
* *