Polishing

spring-context/src/main/java/org/springframework/cache/annotation/SpringCacheAnnotationParser.java

@@ -57,8 +57,7 @@ public class SpringCacheAnnotationParser implements CacheAnnotationParser, Seria
 		return parseCacheAnnotations(defaultConfig, method);
 	}
 
-	protected Collection<CacheOperation> parseCacheAnnotations(DefaultCacheConfig cachingConfig,
+	protected Collection<CacheOperation> parseCacheAnnotations(DefaultCacheConfig cachingConfig, AnnotatedElement ae) {
-															   AnnotatedElement ae) {
 		Collection<CacheOperation> ops = null;
 
 		Collection<Cacheable> cacheables = getAnnotations(ae, Cacheable.class);
@@ -71,24 +70,25 @@ public class SpringCacheAnnotationParser implements CacheAnnotationParser, Seria
 		Collection<CacheEvict> evicts = getAnnotations(ae, CacheEvict.class);
 		if (evicts != null) {
 			ops = lazyInit(ops);
-			for (CacheEvict e : evicts) {
+			for (CacheEvict evict : evicts) {
-				ops.add(parseEvictAnnotation(ae, cachingConfig, e));
+				ops.add(parseEvictAnnotation(ae, cachingConfig, evict));
 			}
 		}
-		Collection<CachePut> updates = getAnnotations(ae, CachePut.class);
+		Collection<CachePut> puts = getAnnotations(ae, CachePut.class);
-		if (updates != null) {
+		if (puts != null) {
 			ops = lazyInit(ops);
-			for (CachePut p : updates) {
+			for (CachePut put : puts) {
-				ops.add(parseUpdateAnnotation(ae, cachingConfig, p));
+				ops.add(parsePutAnnotation(ae, cachingConfig, put));
 			}
 		}
-		Collection<Caching> caching = getAnnotations(ae, Caching.class);
+		Collection<Caching> cachings = getAnnotations(ae, Caching.class);
-		if (caching != null) {
+		if (cachings != null) {
 			ops = lazyInit(ops);
-			for (Caching c : caching) {
+			for (Caching caching : cachings) {
-				ops.addAll(parseCachingAnnotation(ae, cachingConfig, c));
+				ops.addAll(parseCachingAnnotation(ae, cachingConfig, caching));
 			}
 		}
+
 		return ops;
 	}
 
@@ -96,63 +96,62 @@ public class SpringCacheAnnotationParser implements CacheAnnotationParser, Seria
 		return (ops != null ? ops : new ArrayList<CacheOperation>(1));
 	}
 
-	CacheableOperation parseCacheableAnnotation(AnnotatedElement ae,
+	CacheableOperation parseCacheableAnnotation(AnnotatedElement ae, DefaultCacheConfig defaultConfig, Cacheable caching) {
-												DefaultCacheConfig defaultConfig, Cacheable caching) {
+		CacheableOperation op = new CacheableOperation();
-		CacheableOperation cuo = new CacheableOperation();
-		cuo.setCacheNames(caching.value());
-		cuo.setCondition(caching.condition());
-		cuo.setUnless(caching.unless());
-		cuo.setKey(caching.key());
-		cuo.setKeyGenerator(caching.keyGenerator());
-		cuo.setCacheManager(caching.cacheManager());
-		cuo.setCacheResolver(caching.cacheResolver());
-		cuo.setName(ae.toString());
 
-		defaultConfig.applyDefault(cuo);
+		op.setCacheNames(caching.value());
+		op.setCondition(caching.condition());
+		op.setUnless(caching.unless());
+		op.setKey(caching.key());
+		op.setKeyGenerator(caching.keyGenerator());
+		op.setCacheManager(caching.cacheManager());
+		op.setCacheResolver(caching.cacheResolver());
+		op.setName(ae.toString());
 
-		validateCacheOperation(ae, cuo);
+		defaultConfig.applyDefault(op);
-		return cuo;
+		validateCacheOperation(ae, op);
+
+		return op;
 	}
 
-	CacheEvictOperation parseEvictAnnotation(AnnotatedElement ae,
+	CacheEvictOperation parseEvictAnnotation(AnnotatedElement ae, DefaultCacheConfig defaultConfig, CacheEvict caching) {
-											 DefaultCacheConfig defaultConfig, CacheEvict caching) {
+		CacheEvictOperation op = new CacheEvictOperation();
-		CacheEvictOperation ceo = new CacheEvictOperation();
-		ceo.setCacheNames(caching.value());
-		ceo.setCondition(caching.condition());
-		ceo.setKey(caching.key());
-		ceo.setKeyGenerator(caching.keyGenerator());
-		ceo.setCacheManager(caching.cacheManager());
-		ceo.setCacheResolver(caching.cacheResolver());
-		ceo.setCacheWide(caching.allEntries());
-		ceo.setBeforeInvocation(caching.beforeInvocation());
-		ceo.setName(ae.toString());
 
-		defaultConfig.applyDefault(ceo);
+		op.setCacheNames(caching.value());
+		op.setCondition(caching.condition());
+		op.setKey(caching.key());
+		op.setKeyGenerator(caching.keyGenerator());
+		op.setCacheManager(caching.cacheManager());
+		op.setCacheResolver(caching.cacheResolver());
+		op.setCacheWide(caching.allEntries());
+		op.setBeforeInvocation(caching.beforeInvocation());
+		op.setName(ae.toString());
 
-		validateCacheOperation(ae, ceo);
+		defaultConfig.applyDefault(op);
-		return ceo;
+		validateCacheOperation(ae, op);
+
+		return op;
 	}
 
-	CacheOperation parseUpdateAnnotation(AnnotatedElement ae,
+	CacheOperation parsePutAnnotation(AnnotatedElement ae, DefaultCacheConfig defaultConfig, CachePut caching) {
-										 DefaultCacheConfig defaultConfig, CachePut caching) {
+		CachePutOperation op = new CachePutOperation();
-		CachePutOperation cuo = new CachePutOperation();
-		cuo.setCacheNames(caching.value());
-		cuo.setCondition(caching.condition());
-		cuo.setUnless(caching.unless());
-		cuo.setKey(caching.key());
-		cuo.setKeyGenerator(caching.keyGenerator());
-		cuo.setCacheManager(caching.cacheManager());
-		cuo.setCacheResolver(caching.cacheResolver());
-		cuo.setName(ae.toString());
 
-		defaultConfig.applyDefault(cuo);
+		op.setCacheNames(caching.value());
+		op.setCondition(caching.condition());
+		op.setUnless(caching.unless());
+		op.setKey(caching.key());
+		op.setKeyGenerator(caching.keyGenerator());
+		op.setCacheManager(caching.cacheManager());
+		op.setCacheResolver(caching.cacheResolver());
+		op.setName(ae.toString());
 
-		validateCacheOperation(ae, cuo);
+		defaultConfig.applyDefault(op);
-		return cuo;
+		validateCacheOperation(ae, op);
+
+		return op;
 	}
 
-	Collection<CacheOperation> parseCachingAnnotation(AnnotatedElement ae,
+	Collection<CacheOperation> parseCachingAnnotation(AnnotatedElement ae, DefaultCacheConfig defaultConfig, Caching caching) {
-													  DefaultCacheConfig defaultConfig, Caching caching) {
 		Collection<CacheOperation> ops = null;
 
 		Cacheable[] cacheables = caching.cacheable();
@@ -173,7 +172,7 @@ public class SpringCacheAnnotationParser implements CacheAnnotationParser, Seria
 		if (!ObjectUtils.isEmpty(updates)) {
 			ops = lazyInit(ops);
 			for (CachePut update : updates) {
-				ops.add(parseUpdateAnnotation(ae, defaultConfig, update));
+				ops.add(parsePutAnnotation(ae, defaultConfig, update));
 			}
 		}
 
@@ -182,12 +181,11 @@ public class SpringCacheAnnotationParser implements CacheAnnotationParser, Seria
 
 	/**
 	 * Provides the {@link DefaultCacheConfig} instance for the specified {@link Class}.
-	 *
 	 * @param target the class-level to handle
 	 * @return the default config (never {@code null})
 	 */
 	DefaultCacheConfig getDefaultCacheConfig(Class<?> target) {
-		final CacheConfig annotation = AnnotationUtils.getAnnotation(target, CacheConfig.class);
+		CacheConfig annotation = AnnotationUtils.getAnnotation(target, CacheConfig.class);
 		if (annotation != null) {
 			return new DefaultCacheConfig(annotation.cacheNames(), annotation.keyGenerator(),
 					annotation.cacheManager(), annotation.cacheResolver());
@@ -220,27 +218,26 @@ public class SpringCacheAnnotationParser implements CacheAnnotationParser, Seria
 	 * <p>Throws an {@link IllegalStateException} if the state of the operation is
 	 * invalid. As there might be multiple sources for default values, this ensure
 	 * that the operation is in a proper state before being returned.
-	 *
 	 * @param ae the annotated element of the cache operation
 	 * @param operation the {@link CacheOperation} to validate
 	 */
 	private void validateCacheOperation(AnnotatedElement ae, CacheOperation operation) {
 		if (StringUtils.hasText(operation.getKey()) && StringUtils.hasText(operation.getKeyGenerator())) {
-			throw new IllegalStateException("Invalid cache annotation configuration on '"
+			throw new IllegalStateException("Invalid cache annotation configuration on '" +
-					+ ae.toString() + "'. Both 'key' and 'keyGenerator' attributes have been set. " +
+					ae.toString() + "'. Both 'key' and 'keyGenerator' attributes have been set. " +
 					"These attributes are mutually exclusive: either set the SpEL expression used to" +
 					"compute the key at runtime or set the name of the KeyGenerator bean to use.");
 		}
 		if (StringUtils.hasText(operation.getCacheManager()) && StringUtils.hasText(operation.getCacheResolver())) {
-			throw new IllegalStateException("Invalid cache annotation configuration on '"
+			throw new IllegalStateException("Invalid cache annotation configuration on '" +
-					+ ae.toString() + "'. Both 'cacheManager' and 'cacheResolver' attributes have been set. " +
+					ae.toString() + "'. Both 'cacheManager' and 'cacheResolver' attributes have been set. " +
 					"These attributes are mutually exclusive: the cache manager is used to configure a" +
 					"default cache resolver if none is set. If a cache resolver is set, the cache manager" +
 					"won't be used.");
 		}
 		if (operation.getCacheNames().isEmpty()) {
-			throw new IllegalStateException("No cache names could be detected on '"
+			throw new IllegalStateException("No cache names could be detected on '" +
-					+ ae.toString() + "'. Make sure to set the value parameter on the annotation or " +
+					ae.toString() + "'. Make sure to set the value parameter on the annotation or " +
 					"declare a @CacheConfig at the class-level with the default cache name(s) to use.");
 		}
 	}
@@ -260,6 +257,7 @@ public class SpringCacheAnnotationParser implements CacheAnnotationParser, Seria
 	 * Provides default settings for a given set of cache operations.
 	 */
 	static class DefaultCacheConfig {
+
 		private final String[] cacheNames;
 
 		private final String keyGenerator;
@@ -268,47 +266,41 @@ public class SpringCacheAnnotationParser implements CacheAnnotationParser, Seria
 
 		private final String cacheResolver;
 
-		private DefaultCacheConfig(String[] cacheNames, String keyGenerator,
+		public DefaultCacheConfig() {
-				String cacheManager, String cacheResolver) {
+			this(null, null, null, null);
+		}
+
+		private DefaultCacheConfig(String[] cacheNames, String keyGenerator, String cacheManager, String cacheResolver) {
 			this.cacheNames = cacheNames;
 			this.keyGenerator = keyGenerator;
 			this.cacheManager = cacheManager;
 			this.cacheResolver = cacheResolver;
 		}
 
-		public DefaultCacheConfig() {
-			this(null, null, null, null);
-		}
-
 		/**
 		 * Apply the defaults to the specified {@link CacheOperation}.
-		 *
 		 * @param operation the operation to update
 		 */
 		public void applyDefault(CacheOperation operation) {
-			if (operation.getCacheNames().isEmpty() && cacheNames != null) {
+			if (operation.getCacheNames().isEmpty() && this.cacheNames != null) {
-				operation.setCacheNames(cacheNames);
+				operation.setCacheNames(this.cacheNames);
 			}
-			if (!StringUtils.hasText(operation.getKey()) && !StringUtils.hasText(operation.getKeyGenerator())
+			if (!StringUtils.hasText(operation.getKey()) && !StringUtils.hasText(operation.getKeyGenerator()) &&
-					&& StringUtils.hasText(keyGenerator)) {
+					StringUtils.hasText(this.keyGenerator)) {
-				operation.setKeyGenerator(keyGenerator);
+				operation.setKeyGenerator(this.keyGenerator);
 			}
 
-			if (isSet(operation.getCacheManager()) || isSet(operation.getCacheResolver())) {
+			if (StringUtils.hasText(operation.getCacheManager()) || StringUtils.hasText(operation.getCacheResolver())) {
 				// One of these is set so we should not inherit anything
 			}
-			else if (isSet(cacheResolver)) {
+			else if (StringUtils.hasText(this.cacheResolver)) {
-				operation.setCacheResolver(cacheResolver);
+				operation.setCacheResolver(this.cacheResolver);
 			}
-			else if (isSet(cacheManager)) {
+			else if (StringUtils.hasText(this.cacheManager)) {
-				operation.setCacheManager(cacheManager);
+				operation.setCacheManager(this.cacheManager);
 			}
 		}
 
-		private boolean isSet(String s) {
-			return StringUtils.hasText(s);
-		}
-
 	}
 
 }

spring-webmvc/src/main/java/org/springframework/web/servlet/view/AbstractView.java

@@ -227,7 +227,7 @@ public abstract class AbstractView extends WebApplicationObjectSupport implement
 	}
 
 	/**
-	 * Whether to add path variables in the model or not.
+	 * Specify whether to add path variables to the model or not.
 	 * <p>Path variables are commonly bound to URI template variables through the {@code @PathVariable}
 	 * annotation. They're are effectively URI template variables with type conversion applied to
 	 * them to derive typed Object values. Such values are frequently needed in views for
@@ -235,14 +235,14 @@ public abstract class AbstractView extends WebApplicationObjectSupport implement
 	 * <p>Path variables added to the model override static attributes (see {@link #setAttributes(Properties)})
 	 * but not attributes already present in the model.
 	 * <p>By default this flag is set to {@code true}. Concrete view types can override this.
-	 * @param exposePathVariables {@code true} to expose path variables, and {@code false} otherwise.
+	 * @param exposePathVariables {@code true} to expose path variables, and {@code false} otherwise
 	 */
 	public void setExposePathVariables(boolean exposePathVariables) {
 		this.exposePathVariables = exposePathVariables;
 	}
 
 	/**
-	 * Returns the value of the flag indicating whether path variables should be added to the model or not.
+	 * Return whether to add path variables to the model or not.
 	 */
 	public boolean isExposePathVariables() {
 		return this.exposePathVariables;

spring-webmvc/src/main/java/org/springframework/web/servlet/view/InternalResourceView.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -141,7 +141,7 @@ public class InternalResourceView extends AbstractUrlBasedView {
 	 * the {@link #setExposeContextBeansAsAttributes "exposeContextBeansAsAttributes"}
 	 * flag on but do not list specific bean names for this property.
 	 */
-	public void setExposedContextBeanNames(String[] exposedContextBeanNames) {
+	public void setExposedContextBeanNames(String... exposedContextBeanNames) {
 		this.exposedContextBeanNames = new HashSet<String>(Arrays.asList(exposedContextBeanNames));
 	}
 

spring-webmvc/src/main/java/org/springframework/web/servlet/view/InternalResourceViewResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -70,6 +70,7 @@ public class InternalResourceViewResolver extends UrlBasedViewResolver {
 		setViewClass(viewClass);
 	}
 
+
 	/**
 	 * This resolver requires {@link InternalResourceView}.
 	 */
@@ -78,7 +79,6 @@ public class InternalResourceViewResolver extends UrlBasedViewResolver {
 		return InternalResourceView.class;
 	}
 
-
 	/**
 	 * Specify whether to always include the view rather than forward to it.
 	 * <p>Default is "false". Switch this flag on to enforce the use of a
@@ -86,7 +86,7 @@ public class InternalResourceViewResolver extends UrlBasedViewResolver {
 	 * @see InternalResourceView#setAlwaysInclude
 	 */
 	public void setAlwaysInclude(boolean alwaysInclude) {
-		this.alwaysInclude = Boolean.valueOf(alwaysInclude);
+		this.alwaysInclude = alwaysInclude;
 	}
 
 	/**
@@ -108,7 +108,7 @@ public class InternalResourceViewResolver extends UrlBasedViewResolver {
 	 * attributes.
 	 * @see InternalResourceView#setExposedContextBeanNames
 	 */
-	public void setExposedContextBeanNames(String[] exposedContextBeanNames) {
+	public void setExposedContextBeanNames(String... exposedContextBeanNames) {
 		this.exposedContextBeanNames = exposedContextBeanNames;
 	}
 

spring-webmvc/src/main/java/org/springframework/web/servlet/view/UrlBasedViewResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -106,8 +106,6 @@ public class UrlBasedViewResolver extends AbstractCachingViewResolver implements
 
 	private String suffix = "";
 
-	private String[] viewNames = null;
-
 	private String contentType;
 
 	private boolean redirectContextRelative = true;
@@ -116,13 +114,16 @@ public class UrlBasedViewResolver extends AbstractCachingViewResolver implements
 
 	private String requestContextAttribute;
 
-	private int order = Integer.MAX_VALUE;
-
 	/** Map of static attributes, keyed by attribute name (String) */
 	private final Map<String, Object> staticAttributes = new HashMap<String, Object>();
 
 	private Boolean exposePathVariables;
 
+	private String[] viewNames;
+
+	private int order = Integer.MAX_VALUE;
+
+
 	/**
 	 * Set the view class that should be used to create views.
 	 * @param viewClass class that is assignable to the required view class
@@ -303,6 +304,29 @@ public class UrlBasedViewResolver extends AbstractCachingViewResolver implements
 		return this.staticAttributes;
 	}
 
+	/**
+	 * Specify whether views resolved by this resolver should add path variables to the model or not.
+	 * <p>>The default setting is to let each View decide (see {@link AbstractView#setExposePathVariables}.
+	 * However, you can use this property to override that.
+	 * @param exposePathVariables
+	 * <ul>
+	 * <li>{@code true} - all Views resolved by this resolver will expose path variables
+	 * <li>{@code false} - no Views resolved by this resolver will expose path variables
+	 * <li>{@code null} - individual Views can decide for themselves (this is used by the default)
+	 * <ul>
+	 * @see AbstractView#setExposePathVariables
+	 */
+	public void setExposePathVariables(Boolean exposePathVariables) {
+		this.exposePathVariables = exposePathVariables;
+	}
+
+	/**
+	 * Return whether views resolved by this resolver should add path variables to the model or not.
+	 */
+	protected Boolean getExposePathVariables() {
+		return this.exposePathVariables;
+	}
+
 	/**
 	 * Set the view names (or name patterns) that can be handled by this
 	 * {@link org.springframework.web.servlet.ViewResolver}. View names can contain
@@ -310,7 +334,7 @@ public class UrlBasedViewResolver extends AbstractCachingViewResolver implements
 	 * view name 'myReport'.
 	 * @see #canHandle
 	 */
-	public void setViewNames(String[] viewNames) {
+	public void setViewNames(String... viewNames) {
 		this.viewNames = viewNames;
 	}
 
@@ -339,22 +363,6 @@ public class UrlBasedViewResolver extends AbstractCachingViewResolver implements
 		return this.order;
 	}
 
-	/**
-	 * Whether views resolved by this resolver should add path variables the model or not.
-	 * The default setting is to allow each View decide (see {@link AbstractView#setExposePathVariables(boolean)}.
-	 * However, you can use this property to override that.
-	 * @param exposePathVariables
-	 * 	<ul>
-	 * 		<li>{@code true} - all Views resolved by this resolver will expose path variables
-	 * 		<li>{@code false} - no Views resolved by this resolver will expose path variables
-	 * 		<li>{@code null} - individual Views can decide for themselves (this is used by the default)
-	 * 	<ul>
-	 * 	@see AbstractView#setExposePathVariables(boolean)
-	 */
-	public void setExposePathVariables(Boolean exposePathVariables) {
-		this.exposePathVariables = exposePathVariables;
-	}
-
 	@Override
 	protected void initApplicationContext() {
 		super.initApplicationContext();
@@ -363,6 +371,7 @@ public class UrlBasedViewResolver extends AbstractCachingViewResolver implements
 		}
 	}
 
+
 	/**
 	 * This implementation returns just the view name,
 	 * as this ViewResolver doesn't support localized resolution.
@@ -460,15 +469,20 @@ public class UrlBasedViewResolver extends AbstractCachingViewResolver implements
 	protected AbstractUrlBasedView buildView(String viewName) throws Exception {
 		AbstractUrlBasedView view = (AbstractUrlBasedView) BeanUtils.instantiateClass(getViewClass());
 		view.setUrl(getPrefix() + viewName + getSuffix());
+
 		String contentType = getContentType();
 		if (contentType != null) {
 			view.setContentType(contentType);
 		}
+
 		view.setRequestContextAttribute(getRequestContextAttribute());
 		view.setAttributesMap(getAttributesMap());
-		if (this.exposePathVariables != null) {
+
+		Boolean exposePathVariables = getExposePathVariables();
+		if (exposePathVariables != null) {
 			view.setExposePathVariables(exposePathVariables);
 		}
+
 		return view;
 	}