Polishing

spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractFallbackJCacheOperationSource.java

@@ -29,19 +29,18 @@ import org.springframework.core.BridgeMethodResolver;
 import org.springframework.util.ClassUtils;
 
 /**
- * Abstract implementation of {@link JCacheOperationSource} that caches
+ * Abstract implementation of {@link JCacheOperationSource} that caches attributes
- * attributes for methods and implements a fallback policy: 1. specific
+ * for methods and implements a fallback policy: 1. specific target method;
- * target method; 2. declaring method.
+ * 2. declaring method.
  *
- * <p>This implementation caches attributes by method after they are
+ * <p>This implementation caches attributes by method after they are first used.
- * first used.
  *
  * @author Stephane Nicoll
+ * @author Juergen Hoeller
  * @since 4.1
  * @see org.springframework.cache.interceptor.AbstractFallbackCacheOperationSource
  */
-public abstract class AbstractFallbackJCacheOperationSource
+public abstract class AbstractFallbackJCacheOperationSource implements JCacheOperationSource {
-		implements JCacheOperationSource {
 
 	/**
 	 * Canonical value held in cache to indicate no caching attribute was
@@ -49,34 +48,31 @@ public abstract class AbstractFallbackJCacheOperationSource
 	 */
 	private final static Object NULL_CACHING_ATTRIBUTE = new Object();
 
+
 	protected final Log logger = LogFactory.getLog(getClass());
 
-	private final Map<Object, Object> cache =
+	private final Map<Object, Object> cache = new ConcurrentHashMap<Object, Object>(1024);
-			new ConcurrentHashMap<Object, Object>(1024);
+
 
 	@Override
 	public JCacheOperation<?> getCacheOperation(Method method, Class<?> targetClass) {
-		// First, see if we have a cached value.
 		Object cacheKey = new AnnotatedElementKey(method, targetClass);
 		Object cached = this.cache.get(cacheKey);
+
 		if (cached != null) {
-			if (cached == NULL_CACHING_ATTRIBUTE) {
+			return (cached != NULL_CACHING_ATTRIBUTE ? (JCacheOperation<?>) cached : null);
-				return null;
-			}
-			return (JCacheOperation<?>) cached;
 		}
 		else {
 			JCacheOperation<?> operation = computeCacheOperation(method, targetClass);
-			if (operation == null) {
+			if (operation != null) {
-				this.cache.put(cacheKey, NULL_CACHING_ATTRIBUTE);
-			}
-			else {
 				if (logger.isDebugEnabled()) {
-					logger.debug("Adding cacheable method '" + method.getName()
+					logger.debug("Adding cacheable method '" + method.getName() + "' with operation: " + operation);
-							+ "' with operation: " + operation);
 				}
 				this.cache.put(cacheKey, operation);
 			}
+			else {
+				this.cache.put(cacheKey, NULL_CACHING_ATTRIBUTE);
+			}
 			return operation;
 		}
 	}
@@ -108,6 +104,7 @@ public abstract class AbstractFallbackJCacheOperationSource
 		return null;
 	}
 
+
 	/**
 	 * Subclasses need to implement this to return the caching operation
 	 * for the given method, if any.

spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/DefaultJCacheOperationSource.java

@@ -196,12 +196,10 @@ public class DefaultJCacheOperationSource extends AnnotationJCacheOperationSourc
 
 	/**
 	 * Only resolve the default exception cache resolver when an exception needs to be handled.
-	 * <p>
+	 * <p>A non-JSR-107 setup requires either a {@link CacheManager} or a {@link CacheResolver}. If only
-	 * A non-JSR-107 setup requires either a {@link CacheManager} or a {@link CacheResolver}. If only
 	 * the latter is specified, it is not possible to extract a default exception {@code CacheResolver}
 	 * from a custom {@code CacheResolver} implementation so we have to fallback on the {@code CacheManager}.
-	 * <p>
+	 * <p>This gives this weird situation of a perfectly valid configuration that breaks all the sudden
-	 * This gives this weird situation of a perfectly valid configuration that breaks all the sudden
 	 * because the JCache support is enabled. To avoid this we resolve the default exception {@code CacheResolver}
 	 * as late as possible to avoid such hard requirement in other cases.
 	 */

spring-context-support/src/test/java/org/springframework/cache/jcache/config/JCacheJavaConfigTests.java

@@ -61,10 +61,12 @@ public class JCacheJavaConfigTests extends AbstractJCacheAnnotationTests {
 		return new AnnotationConfigApplicationContext(EnableCachingConfig.class);
 	}
 
+
 	@Test
 	public void fullCachingConfig() throws Exception {
 		AnnotationConfigApplicationContext context =
 				new AnnotationConfigApplicationContext(FullCachingConfig.class);
+
 		DefaultJCacheOperationSource cos = context.getBean(DefaultJCacheOperationSource.class);
 		assertSame(context.getBean(KeyGenerator.class), cos.getKeyGenerator());
 		assertSame(context.getBean("cacheResolver", CacheResolver.class),
@@ -103,14 +105,14 @@ public class JCacheJavaConfigTests extends AbstractJCacheAnnotationTests {
 
 	@Test
 	public void exceptionCacheResolverLazilyRequired() {
-		ConfigurableApplicationContext context = new AnnotationConfigApplicationContext(
+		ConfigurableApplicationContext context =
-				NoExceptionCacheResolverConfig.class);
+				new AnnotationConfigApplicationContext(NoExceptionCacheResolverConfig.class);
+
 		try {
 			DefaultJCacheOperationSource cos = context.getBean(DefaultJCacheOperationSource.class);
 			assertSame(context.getBean("cacheResolver"), cos.getCacheResolver());
 
 			JCacheableService<?> service = context.getBean(JCacheableService.class);
-
 			service.cache("id");
 
 			// This call requires the cache manager to be set
@@ -149,11 +151,11 @@ public class JCacheJavaConfigTests extends AbstractJCacheAnnotationTests {
 		}
 	}
 
+
 	@Configuration
 	@EnableCaching
 	public static class FullCachingConfig implements JCacheConfigurer {
 
-
 		@Override
 		@Bean
 		public CacheManager cacheManager() {
@@ -185,6 +187,7 @@ public class JCacheJavaConfigTests extends AbstractJCacheAnnotationTests {
 		}
 	}
 
+
 	@Configuration
 	@EnableCaching
 	public static class EmptyConfigSupportConfig extends JCacheConfigurerSupport {
@@ -194,6 +197,7 @@ public class JCacheJavaConfigTests extends AbstractJCacheAnnotationTests {
 		}
 	}
 
+
 	@Configuration
 	@EnableCaching
 	static class FullCachingConfigSupport extends JCacheConfigurerSupport {
@@ -223,6 +227,7 @@ public class JCacheJavaConfigTests extends AbstractJCacheAnnotationTests {
 		}
 	}
 
+
 	@Configuration
 	@EnableCaching
 	static class NoExceptionCacheResolverConfig extends JCacheConfigurerSupport {

spring-context/src/main/java/org/springframework/cache/interceptor/AbstractFallbackCacheOperationSource.java

@@ -60,6 +60,7 @@ public abstract class AbstractFallbackCacheOperationSource implements CacheOpera
 	 */
 	private final static Collection<CacheOperation> NULL_CACHING_ATTRIBUTE = Collections.emptyList();
 
+
 	/**
 	 * Logger available to subclasses.
 	 * <p>As this base class is not marked Serializable, the logger will be recreated
@@ -68,13 +69,14 @@ public abstract class AbstractFallbackCacheOperationSource implements CacheOpera
 	protected final Log logger = LogFactory.getLog(getClass());
 
 	/**
-	 * Cache of CacheOperations, keyed by {@link MethodCacheKey} (Method + target Class).
+	 * Cache of CacheOperations, keyed by {@link AnnotatedElementKey}.
 	 * <p>As this base class is not marked Serializable, the cache will be recreated
 	 * after serialization - provided that the concrete subclass is Serializable.
 	 */
-	final Map<Object, Collection<CacheOperation>> attributeCache =
+	private final Map<Object, Collection<CacheOperation>> attributeCache =
 			new ConcurrentHashMap<Object, Collection<CacheOperation>>(1024);
 
+
 	/**
 	 * Determine the caching attribute for this method invocation.
 	 * <p>Defaults to the class's caching attribute if no method attribute is found.
@@ -85,30 +87,23 @@ public abstract class AbstractFallbackCacheOperationSource implements CacheOpera
 	 */
 	@Override
 	public Collection<CacheOperation> getCacheOperations(Method method, Class<?> targetClass) {
-		// First, see if we have a cached value.
 		Object cacheKey = getCacheKey(method, targetClass);
 		Collection<CacheOperation> cached = this.attributeCache.get(cacheKey);
+
 		if (cached != null) {
-			if (cached == NULL_CACHING_ATTRIBUTE) {
+			return (cached != NULL_CACHING_ATTRIBUTE ? cached : null);
-				return null;
-			}
-			// Value will either be canonical value indicating there is no caching attribute,
-			// or an actual caching attribute.
-			return cached;
 		}
 		else {
-			// We need to work it out.
 			Collection<CacheOperation> cacheOps = computeCacheOperations(method, targetClass);
-			// Put it in the cache.
+			if (cacheOps != null) {
-			if (cacheOps == null) {
-				this.attributeCache.put(cacheKey, NULL_CACHING_ATTRIBUTE);
-			}
-			else {
 				if (logger.isDebugEnabled()) {
 					logger.debug("Adding cacheable method '" + method.getName() + "' with attribute: " + cacheOps);
 				}
 				this.attributeCache.put(cacheKey, cacheOps);
 			}
+			else {
+				this.attributeCache.put(cacheKey, NULL_CACHING_ATTRIBUTE);
+			}
 			return cacheOps;
 		}
 	}
@@ -187,4 +182,5 @@ public abstract class AbstractFallbackCacheOperationSource implements CacheOpera
 	protected boolean allowPublicMethodsOnly() {
 		return false;
 	}
+
 }

spring-core/src/main/java/org/springframework/core/type/classreading/MethodMetadataReadingVisitor.java

@@ -122,7 +122,7 @@ public class MethodMetadataReadingVisitor extends MethodVisitor implements Metho
 	public AnnotationAttributes getAnnotationAttributes(String annotationName, boolean classValuesAsString) {
 		AnnotationAttributes raw = AnnotationReadingVisitorUtils.getMergedAnnotationAttributes(
 				this.attributesMap, this.metaAnnotationMap, annotationName);
-		return (AnnotationReadingVisitorUtils.convertClassValues(this.classLoader, raw, classValuesAsString));
+		return AnnotationReadingVisitorUtils.convertClassValues(this.classLoader, raw, classValuesAsString);
 	}
 
 	@Override

spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/UndertowXhrTransport.java

@@ -105,8 +105,8 @@ public class UndertowXhrTransport extends AbstractXhrTransport implements XhrTra
 
 	public UndertowXhrTransport(OptionMap optionMap) throws IOException {
 		Assert.notNull(optionMap, "OptionMap is required");
-		this.httpClient = UndertowClient.getInstance();
 		this.optionMap = optionMap;
+		this.httpClient = UndertowClient.getInstance();
 		this.worker = Xnio.getInstance().createWorker(optionMap);
 		this.bufferPool = new ByteBufferSlicePool(1048, 1048);
 	}
@@ -370,7 +370,6 @@ public class UndertowXhrTransport extends AbstractXhrTransport implements XhrTra
 
 		private final ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
 
-
 		public SockJsResponseListener(TransportRequest request, ClientConnection connection, URI url,
 				HttpHeaders headers, XhrClientSockJsSession sockJsSession,
 				SettableListenableFuture<WebSocketSession> connectFuture) {