+ align @CacheEvict behaviour with @Cacheable and @cACHEpUT

+ add flag for post method execution
+ add integration tests

Author: Costin Leau

org.springframework.context/src/main/java/org/springframework/cache/annotation/CacheEvict.java

@@ -63,4 +63,10 @@
 	 */
 	boolean allEntries() default false;
 
+	/**
+	 * Whether the eviction should occur after the method is successfully invoked (default)
+	 * or before. The latter causes the eviction to occur irrespective of the method outcome (whether
+	 * it threw an exception or not) while the former does not.
+	 */
+	boolean afterInvocation() default true;
 }

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

@@ -85,6 +85,7 @@ CacheEvictOperation parseEvictAnnotation(AnnotatedElement ae, CacheEvict caching
 		ceo.setCondition(caching.condition());
 		ceo.setKey(caching.key());
 		ceo.setCacheWide(caching.allEntries());
+		ceo.setAfterInvocation(caching.afterInvocation());
 		ceo.setName(ae.toString());
 		return ceo;
 	}

org.springframework.context/src/main/java/org/springframework/cache/config/CacheAdviceParser.java

@@ -68,7 +68,7 @@ private static class Props {
 			}
 		}
 
-		CacheOperation merge(Element element, ReaderContext readerCtx, CacheOperation op) {
+		<T extends CacheOperation> T merge(Element element, ReaderContext readerCtx, T op) {
 			String cache = element.getAttribute("cache");
 			String k = element.getAttribute("key");
 			String c = element.getAttribute("condition");
@@ -181,7 +181,17 @@ private RootBeanDefinition parseDefinitionSource(Element definition, ParserConte
 			String name = prop.merge(opElement, parserContext.getReaderContext());
 			TypedStringValue nameHolder = new TypedStringValue(name);
 			nameHolder.setSource(parserContext.extractSource(opElement));
-			CacheOperation op = prop.merge(opElement, parserContext.getReaderContext(), new CacheEvictOperation());
+			CacheEvictOperation op = prop.merge(opElement, parserContext.getReaderContext(), new CacheEvictOperation());
+
+			String wide = opElement.getAttribute("all-entries");
+			if (StringUtils.hasText(wide)) {
+				op.setCacheWide(Boolean.valueOf(wide.trim()));
+			}
+
+			String after = opElement.getAttribute("after-invocation");
+			if (StringUtils.hasText(after)) {
+				op.setAfterInvocation(Boolean.valueOf(after.trim()));
+			}
 
 			Collection<CacheOperation> col = cacheOpMap.get(nameHolder);
 			if (col == null) {

org.springframework.context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java

@@ -189,11 +189,10 @@ protected Object execute(Invoker invoker, Object target, Method method, Object[]
 
 		// analyze caching information
 		if (!CollectionUtils.isEmpty(cacheOp)) {
-			Map<String, Collection<CacheOperationContext>> ops = createOperationContext(cacheOp, method, args, target,
-					targetClass);
+			Map<String, Collection<CacheOperationContext>> ops = createOperationContext(cacheOp, method, args, target, targetClass);
 
 			// start with evictions
-			inspectCacheEvicts(ops.get(EVICT));
+			inspectBeforeCacheEvicts(ops.get(EVICT));
 
 			// follow up with cacheable
 			CacheStatus status = inspectCacheables(ops.get(CACHEABLE));
@@ -213,6 +212,8 @@ protected Object execute(Invoker invoker, Object target, Method method, Object[]
 
 			retVal = invoker.invoke();
 
+			inspectAfterCacheEvicts(ops.get(EVICT));
+
 			if (!updates.isEmpty()) {
 				update(updates, retVal);
 			}
@@ -223,42 +224,51 @@ protected Object execute(Invoker invoker, Object target, Method method, Object[]
 		return invoker.invoke();
 	}
 
-	private void inspectCacheEvicts(Collection<CacheOperationContext> evictions) {
+	private void inspectBeforeCacheEvicts(Collection<CacheOperationContext> evictions) {
+		inspectAfterCacheEvicts(evictions, false);
+	}
+
+	private void inspectAfterCacheEvicts(Collection<CacheOperationContext> evictions) {
+		inspectAfterCacheEvicts(evictions, true);
+	}
+
+	private void inspectAfterCacheEvicts(Collection<CacheOperationContext> evictions, boolean afterInvocation) {
 
 		if (!evictions.isEmpty()) {
 
 			boolean log = logger.isTraceEnabled();
 
 			for (CacheOperationContext context : evictions) {
-				if (context.isConditionPassing()) {
-					CacheEvictOperation evictOp = (CacheEvictOperation) context.operation;
-
-					// for each cache
-					// lazy key initialization
-					Object key = null;
-
-					for (Cache cache : context.getCaches()) {
-						// cache-wide flush
-						if (evictOp.isCacheWide()) {
-							cache.clear();
-							if (log) {
-								logger.trace("Invalidating entire cache for operation " + evictOp + " on method " + context.method);
-							}
-						} else {
-							// check key
-							if (key == null) {
-								key = context.generateKey();
-							}
-							if (log) {
-								logger.trace("Invalidating cache key " + key + " for operation " + evictOp + " on method " + context.method);
+				CacheEvictOperation evictOp = (CacheEvictOperation) context.operation;
+
+				if (afterInvocation == evictOp.isAfterInvocation()) {
+					if (context.isConditionPassing()) {
+						// for each cache
+						// lazy key initialization
+						Object key = null;
+
+						for (Cache cache : context.getCaches()) {
+							// cache-wide flush
+							if (evictOp.isCacheWide()) {
+								cache.clear();
+								if (log) {
+									logger.trace("Invalidating entire cache for operation " + evictOp + " on method " + context.method);
+								}
+							} else {
+								// check key
+								if (key == null) {
+									key = context.generateKey();
+								}
+								if (log) {
+									logger.trace("Invalidating cache key " + key + " for operation " + evictOp + " on method " + context.method);
+								}
+								cache.evict(key);
 							}
-							cache.evict(key);
 						}
-					}
-				}
-				else {
-					if (log) {
-						logger.trace("Cache condition failed on method " + context.method + " for operation " + context.operation);
+					} else {
+						if (log) {
+							logger.trace("Cache condition failed on method " + context.method + " for operation " + context.operation);
+						}
 					}
 				}
 			}

org.springframework.context/src/main/java/org/springframework/cache/interceptor/CacheEvictOperation.java

@@ -25,6 +25,7 @@
 public class CacheEvictOperation extends CacheOperation {
 
 	private boolean cacheWide = false;
+	private boolean afterInvocation = true;
 
 	public void setCacheWide(boolean cacheWide) {
 		this.cacheWide = cacheWide;
@@ -34,11 +35,21 @@ public boolean isCacheWide() {
 		return this.cacheWide;
 	}
 
+	public void setAfterInvocation(boolean afterInvocation) {
+		this.afterInvocation = afterInvocation;
+	}
+
+	public boolean isAfterInvocation() {
+		return this.afterInvocation;
+	}
+
 	@Override
 	protected StringBuilder getOperationDescription() {
 		StringBuilder sb = super.getOperationDescription();
 		sb.append(",");
 		sb.append(this.cacheWide);
+		sb.append(",");
+		sb.append(this.afterInvocation);
 		return sb;
 	}
 }

org.springframework.context/src/main/resources/org/springframework/cache/config/spring-cache-3.1.xsd

@@ -211,9 +211,17 @@
 									<xsd:attribute name="all-entries" type="xsd:boolean" use="optional">
 										<xsd:annotation>
 											<xsd:documentation><![CDATA[
-	The name of the backing cache.]]></xsd:documentation>
+	Whether all the entries should be evicted.]]></xsd:documentation>
 										</xsd:annotation>
 									</xsd:attribute>
+									<xsd:attribute name="after-invocation" type="xsd:boolean" use="optional">
+										<xsd:annotation>
+											<xsd:documentation><![CDATA[
+	Whether the eviction should occur after the method is successfully 
+	invoked (default) or before.]]></xsd:documentation>
+										</xsd:annotation>
+									</xsd:attribute>
+									
 								</xsd:extension>
 							</xsd:complexContent>
 						</xsd:complexType>