+ align @CacheEvict behaviour with @Cacheable and @CachePut

+ add flag for post method execution
+ add integration tests

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

@@ -63,4 +63,10 @@ public @interface CacheEvict {
 	 */
 	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 @@ public class SpringCacheAnnotationParser implements CacheAnnotationParser, Seria
 		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 @@ class CacheAdviceParser extends AbstractSingleBeanDefinitionParser {
 			}
 		}
 
-		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 @@ class CacheAdviceParser extends AbstractSingleBeanDefinitionParser {
 			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 @@ public abstract class CacheAspectSupport implements InitializingBean {
 
 		// 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 @@ public abstract class CacheAspectSupport implements InitializingBean {
 
 			retVal = invoker.invoke();
 
+			inspectAfterCacheEvicts(ops.get(EVICT));
+
 			if (!updates.isEmpty()) {
 				update(updates, retVal);
 			}
@@ -223,42 +224,51 @@ public abstract class CacheAspectSupport implements InitializingBean {
 		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 @@ package org.springframework.cache.interceptor;
 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 class CacheEvictOperation extends CacheOperation {
 		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>

org.springframework.context/src/test/java/org/springframework/cache/config/AbstractAnnotationTests.java

@@ -16,13 +16,7 @@
 
 package org.springframework.cache.config;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNotSame;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.*;
 
 import java.util.Collection;
 import java.util.UUID;
@@ -76,7 +70,7 @@ public abstract class AbstractAnnotationTests {
 		assertSame(r1, r3);
 	}
 
-	public void testInvalidate(CacheableService<?> service) throws Exception {
+	public void testEvict(CacheableService<?> service) throws Exception {
 		Object o1 = new Object();
 
 		Object r1 = service.cache(o1);
@@ -90,7 +84,43 @@ public abstract class AbstractAnnotationTests {
 		assertSame(r3, r4);
 	}
 
-	public void testInvalidateWKey(CacheableService<?> service) throws Exception {
+	public void testEvictEarly(CacheableService<?> service) throws Exception {
+		Object o1 = new Object();
+
+		Object r1 = service.cache(o1);
+		Object r2 = service.cache(o1);
+
+		assertSame(r1, r2);
+		try {
+			service.evictEarly(o1);
+		} catch (RuntimeException ex) {
+			// expected
+		}
+
+		Object r3 = service.cache(o1);
+		Object r4 = service.cache(o1);
+		assertNotSame(r1, r3);
+		assertSame(r3, r4);
+	}
+
+	public void testEvictException(CacheableService<?> service) throws Exception {
+		Object o1 = new Object();
+
+		Object r1 = service.cache(o1);
+		Object r2 = service.cache(o1);
+
+		assertSame(r1, r2);
+		try {
+			service.evictWithException(o1);
+		} catch (RuntimeException ex) {
+			// expected
+		}
+		// exception occurred, eviction skipped, data should still be in the cache
+		Object r3 = service.cache(o1);
+		assertSame(r1, r3);
+	}
+
+	public void testEvictWKey(CacheableService<?> service) throws Exception {
 		Object o1 = new Object();
 
 		Object r1 = service.cache(o1);
@@ -104,8 +134,48 @@ public abstract class AbstractAnnotationTests {
 		assertSame(r3, r4);
 	}
 
-	public void testConditionalExpression(CacheableService<?> service)
-			throws Exception {
+	public void testEvictWKeyEarly(CacheableService<?> service) throws Exception {
+		Object o1 = new Object();
+
+		Object r1 = service.cache(o1);
+		Object r2 = service.cache(o1);
+
+		assertSame(r1, r2);
+
+		try {
+			service.invalidateEarly(o1, null);
+		} catch (Exception ex) {
+			// expected
+		}
+		Object r3 = service.cache(o1);
+		Object r4 = service.cache(o1);
+		assertNotSame(r1, r3);
+		assertSame(r3, r4);
+	}
+
+	public void testEvictAll(CacheableService<?> service) throws Exception {
+		Object o1 = new Object();
+
+		Object r1 = service.cache(o1);
+		Object r2 = service.cache(o1);
+
+		Object o2 = new Object();
+		Object r10 = service.cache(o2);
+
+		assertSame(r1, r2);
+		assertNotSame(r1, r10);
+		service.evictAll(new Object());
+		Cache cache = cm.getCache("default");
+		assertNull(cache.get(o1));
+		assertNull(cache.get(o2));
+
+		Object r3 = service.cache(o1);
+		Object r4 = service.cache(o1);
+		assertNotSame(r1, r3);
+		assertSame(r3, r4);
+	}
+
+	public void testConditionalExpression(CacheableService<?> service) throws Exception {
 		Object r1 = service.conditional(4);
 		Object r2 = service.conditional(4);
 
@@ -139,8 +209,7 @@ public abstract class AbstractAnnotationTests {
 		assertEquals(nr + 1, service.nullInvocations().intValue());
 	}
 
-	public void testMethodName(CacheableService<?> service, String keyName)
-			throws Exception {
+	public void testMethodName(CacheableService<?> service, String keyName) throws Exception {
 		Object key = new Object();
 		Object r1 = service.name(key);
 		assertSame(r1, service.name(key));
@@ -335,12 +404,32 @@ public abstract class AbstractAnnotationTests {
 
 	@Test
 	public void testInvalidate() throws Exception {
-		testInvalidate(cs);
+		testEvict(cs);
+	}
+
+	@Test
+	public void testEarlyInvalidate() throws Exception {
+		testEvictEarly(cs);
+	}
+
+	@Test
+	public void testEvictWithException() throws Exception {
+		testEvictException(cs);
+	}
+
+	@Test
+	public void testEvictAll() throws Exception {
+		testEvictAll(cs);
 	}
 
 	@Test
 	public void testInvalidateWithKey() throws Exception {
-		testInvalidateWKey(cs);
+		testEvictWKey(cs);
+	}
+
+	@Test
+	public void testEarlyInvalidateWithKey() throws Exception {
+		testEvictWKeyEarly(cs);
 	}
 
 	@Test
@@ -360,12 +449,32 @@ public abstract class AbstractAnnotationTests {
 
 	@Test
 	public void testClassCacheInvalidate() throws Exception {
-		testInvalidate(ccs);
+		testEvict(ccs);
+	}
+
+	@Test
+	public void testClassEarlyInvalidate() throws Exception {
+		testEvictEarly(ccs);
+	}
+
+	@Test
+	public void testClassEvictAll() throws Exception {
+		testEvictAll(ccs);
+	}
+
+	@Test
+	public void testClassEvictWithException() throws Exception {
+		testEvictException(ccs);
 	}
 
 	@Test
 	public void testClassCacheInvalidateWKey() throws Exception {
-		testInvalidateWKey(ccs);
+		testEvictWKey(ccs);
+	}
+
+	@Test
+	public void testClassEarlyInvalidateWithKey() throws Exception {
+		testEvictWKeyEarly(ccs);
 	}
 
 	@Test
@@ -383,8 +492,7 @@ public abstract class AbstractAnnotationTests {
 		assertNull(ccs.nullValue(new Object()));
 		// the check method is also cached
 		assertEquals(nr, ccs.nullInvocations().intValue());
-		assertEquals(nr + 1, AnnotatedClassCacheableService.nullInvocations
-				.intValue());
+		assertEquals(nr + 1, AnnotatedClassCacheableService.nullInvocations.intValue());
 	}
 
 	@Test

org.springframework.context/src/test/java/org/springframework/cache/config/AnnotatedClassCacheableService.java

@@ -18,10 +18,10 @@ package org.springframework.cache.config;
 
 import java.util.concurrent.atomic.AtomicLong;
 
-import org.springframework.cache.annotation.Caching;
 import org.springframework.cache.annotation.CacheEvict;
 import org.springframework.cache.annotation.CachePut;
 import org.springframework.cache.annotation.Cacheable;
+import org.springframework.cache.annotation.Caching;
 
 /**
  * @author Costin Leau
@@ -44,10 +44,29 @@ public class AnnotatedClassCacheableService implements CacheableService<Object>
 	public void invalidate(Object arg1) {
 	}
 
+	@CacheEvict("default")
+	public void evictWithException(Object arg1) {
+		throw new RuntimeException("exception thrown - evict should NOT occur");
+	}
+
+	@CacheEvict(value = "default", allEntries = true)
+	public void evictAll(Object arg1) {
+	}
+
+	@CacheEvict(value = "default", afterInvocation = false)
+	public void evictEarly(Object arg1) {
+		throw new RuntimeException("exception thrown - evict should still occur");
+	}
+
 	@CacheEvict(value = "default", key = "#p0")
 	public void evict(Object arg1, Object arg2) {
 	}
 
+	@CacheEvict(value = "default", key = "#p0", afterInvocation = false)
+	public void invalidateEarly(Object arg1, Object arg2) {
+		throw new RuntimeException("exception thrown - evict should still occur");
+	}
+
 	@Cacheable(value = "default", key = "#p0")
 	public Object key(Object arg1, Object arg2) {
 		return counter.getAndIncrement();

org.springframework.context/src/test/java/org/springframework/cache/config/CacheableService.java

@@ -27,8 +27,16 @@ public interface CacheableService<T> {
 
 	void invalidate(Object arg1);
 
+	void evictEarly(Object arg1);
+
+	void evictAll(Object arg1);
+
+	void evictWithException(Object arg1);
+
 	void evict(Object arg1, Object arg2);
 
+	void invalidateEarly(Object arg1, Object arg2);
+
 	T conditional(int field);
 
 	T key(Object arg1, Object arg2);

org.springframework.context/src/test/java/org/springframework/cache/config/DefaultCacheableService.java

@@ -18,10 +18,10 @@ package org.springframework.cache.config;
 
 import java.util.concurrent.atomic.AtomicLong;
 
-import org.springframework.cache.annotation.Caching;
 import org.springframework.cache.annotation.CacheEvict;
 import org.springframework.cache.annotation.CachePut;
 import org.springframework.cache.annotation.Cacheable;
+import org.springframework.cache.annotation.Caching;
 
 /**
  * Simple cacheable service
@@ -42,10 +42,29 @@ public class DefaultCacheableService implements CacheableService<Long> {
 	public void invalidate(Object arg1) {
 	}
 
+	@CacheEvict("default")
+	public void evictWithException(Object arg1) {
+		throw new RuntimeException("exception thrown - evict should NOT occur");
+	}
+
+	@CacheEvict(value = "default", allEntries = true)
+	public void evictAll(Object arg1) {
+	}
+
+	@CacheEvict(value = "default", afterInvocation = false)
+	public void evictEarly(Object arg1) {
+		throw new RuntimeException("exception thrown - evict should still occur");
+	}
+
 	@CacheEvict(value = "default", key = "#p0")
 	public void evict(Object arg1, Object arg2) {
 	}
 
+	@CacheEvict(value = "default", key = "#p0", afterInvocation = false)
+	public void invalidateEarly(Object arg1, Object arg2) {
+		throw new RuntimeException("exception thrown - evict should still occur");
+	}
+
 	@Cacheable(value = "default", condition = "#classField == 3")
 	public Long conditional(int classField) {
 		return counter.getAndIncrement();

org.springframework.context/src/test/resources/org/springframework/cache/config/cache-advice.xml

@@ -19,6 +19,10 @@
 		<cache:caching>
 			<cache:cache-evict method="invalidate" cache="default"/>
 			<cache:cache-evict method="evict" key="#p0" cache="default"/>
+			<cache:cache-evict method="evictWithException" cache="default"/>
+			<cache:cache-evict method="evictEarly" cache="default" after-invocation="false"/>
+			<cache:cache-evict method="invalidateEarly" key="#p0" cache="default" after-invocation="false"/>
+			<cache:cache-evict method="evictAll" cache="default" all-entries="true"/>
 		</cache:caching>
 		<cache:caching cache="default">
 			<cache:cache-put method="update"/>
@@ -54,6 +58,10 @@
 		<cache:caching>
 			<cache:cache-evict method="invalidate" cache="default"/>
 			<cache:cache-evict method="evict" key="#p0" cache="default"/>
+			<cache:cache-evict method="evictWithException" cache="default"/>
+			<cache:cache-evict method="evictEarly" cache="default" after-invocation="false"/>
+			<cache:cache-evict method="invalidateEarly" key="#p0" cache="default" after-invocation="false"/>
+			<cache:cache-evict method="evictAll" cache="default" all-entries="true"/>
 		</cache:caching>
 		<cache:caching cache="default">
 			<cache:cache-put method="update"/>