From e05fb494f556d8797bade1e8cb0a03737024ba5b Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 26 Aug 2015 11:04:14 +0200 Subject: [PATCH] Polishing --- ...AbstractFallbackJCacheOperationSource.java | 37 +++++++++---------- .../DefaultJCacheOperationSource.java | 6 +-- .../jcache/config/JCacheJavaConfigTests.java | 13 +++++-- .../AbstractFallbackCacheOperationSource.java | 26 ++++++------- .../MethodMetadataReadingVisitor.java | 2 +- .../sockjs/client/UndertowXhrTransport.java | 3 +- 6 files changed, 41 insertions(+), 46 deletions(-) diff --git a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractFallbackJCacheOperationSource.java b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractFallbackJCacheOperationSource.java index 88813c3cda2..3f18c3c878f 100644 --- a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractFallbackJCacheOperationSource.java +++ b/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 - * attributes for methods and implements a fallback policy: 1. specific - * target method; 2. declaring method. + * Abstract implementation of {@link JCacheOperationSource} that caches attributes + * for methods and implements a fallback policy: 1. specific target method; + * 2. declaring method. * - *

This implementation caches attributes by method after they are - * first used. + *

This implementation caches attributes by method after they are first used. * * @author Stephane Nicoll + * @author Juergen Hoeller * @since 4.1 * @see org.springframework.cache.interceptor.AbstractFallbackCacheOperationSource */ -public abstract class AbstractFallbackJCacheOperationSource - implements JCacheOperationSource { +public abstract class AbstractFallbackJCacheOperationSource 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 cache = - new ConcurrentHashMap(1024); + private final Map cache = new ConcurrentHashMap(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 null; - } - return (JCacheOperation) cached; + return (cached != NULL_CACHING_ATTRIBUTE ? (JCacheOperation) cached : null); } else { JCacheOperation operation = computeCacheOperation(method, targetClass); - if (operation == null) { - this.cache.put(cacheKey, NULL_CACHING_ATTRIBUTE); - } - else { + if (operation != null) { if (logger.isDebugEnabled()) { - logger.debug("Adding cacheable method '" + method.getName() - + "' with operation: " + operation); + logger.debug("Adding cacheable method '" + method.getName() + "' 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. diff --git a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/DefaultJCacheOperationSource.java b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/DefaultJCacheOperationSource.java index f320d6f2dc1..acc1a12ec72 100644 --- a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/DefaultJCacheOperationSource.java +++ b/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. - *

- * 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}. - *

- * 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. */ diff --git a/spring-context-support/src/test/java/org/springframework/cache/jcache/config/JCacheJavaConfigTests.java b/spring-context-support/src/test/java/org/springframework/cache/jcache/config/JCacheJavaConfigTests.java index 7ac5434b0d1..1c72a2a433a 100644 --- a/spring-context-support/src/test/java/org/springframework/cache/jcache/config/JCacheJavaConfigTests.java +++ b/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( - NoExceptionCacheResolverConfig.class); + ConfigurableApplicationContext context = + 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 { diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/AbstractFallbackCacheOperationSource.java b/spring-context/src/main/java/org/springframework/cache/interceptor/AbstractFallbackCacheOperationSource.java index 7308d562762..d383b281116 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/AbstractFallbackCacheOperationSource.java +++ b/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 NULL_CACHING_ATTRIBUTE = Collections.emptyList(); + /** * Logger available to subclasses. *

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}. *

As this base class is not marked Serializable, the cache will be recreated * after serialization - provided that the concrete subclass is Serializable. */ - final Map> attributeCache = + private final Map> attributeCache = new ConcurrentHashMap>(1024); + /** * Determine the caching attribute for this method invocation. *

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 getCacheOperations(Method method, Class targetClass) { - // First, see if we have a cached value. Object cacheKey = getCacheKey(method, targetClass); Collection cached = this.attributeCache.get(cacheKey); + if (cached != null) { - if (cached == NULL_CACHING_ATTRIBUTE) { - return null; - } - // Value will either be canonical value indicating there is no caching attribute, - // or an actual caching attribute. - return cached; + return (cached != NULL_CACHING_ATTRIBUTE ? cached : null); } else { - // We need to work it out. Collection cacheOps = computeCacheOperations(method, targetClass); - // Put it in the cache. - if (cacheOps == null) { - this.attributeCache.put(cacheKey, NULL_CACHING_ATTRIBUTE); - } - else { + if (cacheOps != null) { 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; } + } diff --git a/spring-core/src/main/java/org/springframework/core/type/classreading/MethodMetadataReadingVisitor.java b/spring-core/src/main/java/org/springframework/core/type/classreading/MethodMetadataReadingVisitor.java index 155b8ca25d7..00bd9cb1303 100644 --- a/spring-core/src/main/java/org/springframework/core/type/classreading/MethodMetadataReadingVisitor.java +++ b/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 diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/UndertowXhrTransport.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/UndertowXhrTransport.java index 885843f84c3..ad475eef372 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/UndertowXhrTransport.java +++ b/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 connectFuture) {