Revised PersistenceExceptionTranslationInterceptor to lazily retrieve PersistenceExceptionTranslator beans on demand

Issue: SPR-10894

spring-tx/src/main/java/org/springframework/dao/support/PersistenceExceptionTranslationInterceptor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2013 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.
@@ -47,10 +47,12 @@ import org.springframework.util.ReflectionUtils;
 public class PersistenceExceptionTranslationInterceptor
 		implements MethodInterceptor, BeanFactoryAware, InitializingBean {
 
-	private PersistenceExceptionTranslator persistenceExceptionTranslator;
+	private volatile PersistenceExceptionTranslator persistenceExceptionTranslator;
 
 	private boolean alwaysTranslate = false;
 
+	private ListableBeanFactory beanFactory;
+
 
 	/**
 	 * Create a new PersistenceExceptionTranslationInterceptor.
@@ -63,10 +65,11 @@ public class PersistenceExceptionTranslationInterceptor
 	/**
 	 * Create a new PersistenceExceptionTranslationInterceptor
 	 * for the given PersistenceExceptionTranslator.
-	 * @param persistenceExceptionTranslator the PersistenceExceptionTranslator to use
+	 * @param pet the PersistenceExceptionTranslator to use
 	 */
-	public PersistenceExceptionTranslationInterceptor(PersistenceExceptionTranslator persistenceExceptionTranslator) {
-		setPersistenceExceptionTranslator(persistenceExceptionTranslator);
+	public PersistenceExceptionTranslationInterceptor(PersistenceExceptionTranslator pet) {
+		Assert.notNull(pet, "PersistenceExceptionTranslator must not be null");
+		this.persistenceExceptionTranslator = pet;
 	}
 
 	/**
@@ -76,7 +79,8 @@ public class PersistenceExceptionTranslationInterceptor
 	 * PersistenceExceptionTranslators from
 	 */
 	public PersistenceExceptionTranslationInterceptor(ListableBeanFactory beanFactory) {
-		this.persistenceExceptionTranslator = detectPersistenceExceptionTranslators(beanFactory);
+		Assert.notNull(beanFactory, "ListableBeanFactory must not be null");
+		this.beanFactory = beanFactory;
 	}
 
 
@@ -87,7 +91,6 @@ public class PersistenceExceptionTranslationInterceptor
 	 * @see #detectPersistenceExceptionTranslators
 	 */
 	public void setPersistenceExceptionTranslator(PersistenceExceptionTranslator pet) {
-		Assert.notNull(pet, "PersistenceExceptionTranslator must not be null");
 		this.persistenceExceptionTranslator = pet;
 	}
 
@@ -115,19 +118,37 @@ public class PersistenceExceptionTranslationInterceptor
 				throw new IllegalArgumentException(
 						"Cannot use PersistenceExceptionTranslator autodetection without ListableBeanFactory");
 			}
-			this.persistenceExceptionTranslator =
-					detectPersistenceExceptionTranslators((ListableBeanFactory) beanFactory);
+			this.beanFactory = (ListableBeanFactory) beanFactory;
 		}
 	}
 
 	@Override
 	public void afterPropertiesSet() {
-		if (this.persistenceExceptionTranslator == null) {
+		if (this.persistenceExceptionTranslator == null && this.beanFactory == null) {
 			throw new IllegalArgumentException("Property 'persistenceExceptionTranslator' is required");
 		}
 	}
 
 
+	@Override
+	public Object invoke(MethodInvocation mi) throws Throwable {
+		try {
+			return mi.proceed();
+		}
+		catch (RuntimeException ex) {
+			// Let it throw raw if the type of the exception is on the throws clause of the method.
+			if (!this.alwaysTranslate && ReflectionUtils.declaresException(mi.getMethod(), ex.getClass())) {
+				throw ex;
+			}
+			else {
+				if (this.persistenceExceptionTranslator == null) {
+					this.persistenceExceptionTranslator = detectPersistenceExceptionTranslators(this.beanFactory);
+				}
+				throw DataAccessUtils.translateIfNecessary(ex, this.persistenceExceptionTranslator);
+			}
+		}
+	}
+
 	/**
 	 * Detect all PersistenceExceptionTranslators in the given BeanFactory.
 	 * @param beanFactory the ListableBeanFactory to obtaining all
@@ -140,10 +161,6 @@ public class PersistenceExceptionTranslationInterceptor
 		// Find all translators, being careful not to activate FactoryBeans.
 		Map<String, PersistenceExceptionTranslator> pets = BeanFactoryUtils.beansOfTypeIncludingAncestors(
 				beanFactory, PersistenceExceptionTranslator.class, false, false);
-		if (pets.isEmpty()) {
-			throw new IllegalStateException(
-					"No persistence exception translators found in bean factory. Cannot perform exception translation.");
-		}
 		ChainedPersistenceExceptionTranslator cpet = new ChainedPersistenceExceptionTranslator();
 		for (PersistenceExceptionTranslator pet : pets.values()) {
 			cpet.addDelegate(pet);
@@ -151,21 +168,4 @@ public class PersistenceExceptionTranslationInterceptor
 		return cpet;
 	}
 
-
-	@Override
-	public Object invoke(MethodInvocation mi) throws Throwable {
-		try {
-			return mi.proceed();
-		}
-		catch (RuntimeException ex) {
-			// Let it throw raw if the type of the exception is on the throws clause of the method.
-			if (!this.alwaysTranslate && ReflectionUtils.declaresException(mi.getMethod(), ex.getClass())) {
-				throw ex;
-			}
-			else {
-				throw DataAccessUtils.translateIfNecessary(ex, this.persistenceExceptionTranslator);
-			}
-		}
-	}
-
 }

spring-tx/src/test/java/org/springframework/dao/annotation/PersistenceExceptionTranslationPostProcessorTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2013 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.
@@ -16,6 +16,8 @@
 
 package org.springframework.dao.annotation;
 
+import javax.persistence.PersistenceException;
+
 import junit.framework.TestCase;
 import org.aspectj.lang.JoinPoint;
 import org.aspectj.lang.annotation.Aspect;
@@ -25,38 +27,23 @@ import org.springframework.aop.Advisor;
 import org.springframework.aop.aspectj.annotation.AnnotationAwareAspectJAutoProxyCreator;
 import org.springframework.aop.framework.Advised;
 import org.springframework.aop.support.AopUtils;
-import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.support.RootBeanDefinition;
 import org.springframework.context.support.GenericApplicationContext;
+import org.springframework.dao.DataAccessException;
+import org.springframework.dao.DataAccessResourceFailureException;
 import org.springframework.dao.annotation.PersistenceExceptionTranslationAdvisorTests.RepositoryInterface;
 import org.springframework.dao.annotation.PersistenceExceptionTranslationAdvisorTests.RepositoryInterfaceImpl;
 import org.springframework.dao.annotation.PersistenceExceptionTranslationAdvisorTests.StereotypedRepositoryInterfaceImpl;
-import org.springframework.dao.support.ChainedPersistenceExceptionTranslator;
+import org.springframework.dao.support.PersistenceExceptionTranslator;
 import org.springframework.stereotype.Repository;
 
 /**
- * Unit tests for PersistenceExceptionTranslationPostProcessor. Does not test translation; there are separate unit tests
- * for the Spring AOP Advisor. Just checks whether proxying occurs correctly, as a unit test should.
- *
  * @author Rod Johnson
+ * @author Juergen Hoeller
  */
 public class PersistenceExceptionTranslationPostProcessorTests extends TestCase {
 
-	public void testFailsWithNoPersistenceExceptionTranslators() {
-		GenericApplicationContext gac = new GenericApplicationContext();
-		gac.registerBeanDefinition("translator",
-				new RootBeanDefinition(PersistenceExceptionTranslationPostProcessor.class));
-		gac.registerBeanDefinition("proxied", new RootBeanDefinition(StereotypedRepositoryInterfaceImpl.class));
-		try {
-			gac.refresh();
-			fail("Should fail with no translators");
-		}
-		catch (BeansException ex) {
-			// Ok
-		}
-	}
-
 	public void testProxiesCorrectly() {
 		GenericApplicationContext gac = new GenericApplicationContext();
 		gac.registerBeanDefinition("translator",
@@ -66,8 +53,8 @@ public class PersistenceExceptionTranslationPostProcessorTests extends TestCase
 		gac.registerBeanDefinition("classProxied", new RootBeanDefinition(RepositoryWithoutInterface.class));
 		gac.registerBeanDefinition("classProxiedAndAdvised",
 				new RootBeanDefinition(RepositoryWithoutInterfaceAndOtherwiseAdvised.class));
-		gac.registerBeanDefinition("chainedTranslator",
-				new RootBeanDefinition(ChainedPersistenceExceptionTranslator.class));
+		gac.registerBeanDefinition("myTranslator",
+				new RootBeanDefinition(MyPersistenceExceptionTranslator.class));
 		gac.registerBeanDefinition("proxyCreator",
 				BeanDefinitionBuilder.rootBeanDefinition(AnnotationAwareAspectJAutoProxyCreator.class).
 						addPropertyValue("order", 50).getBeanDefinition());
@@ -84,8 +71,15 @@ public class PersistenceExceptionTranslationPostProcessorTests extends TestCase
 
 		Additional rwi2 = (Additional) gac.getBean("classProxiedAndAdvised");
 		assertTrue(AopUtils.isAopProxy(rwi2));
-		rwi2.additionalMethod();
+		rwi2.additionalMethod(false);
 		checkWillTranslateExceptions(rwi2);
+		try {
+			rwi2.additionalMethod(true);
+			fail("Should have thrown DataAccessResourceFailureException");
+		}
+		catch (DataAccessResourceFailureException ex) {
+			assertEquals("my failure", ex.getMessage());
+		}
 	}
 
 	protected void checkWillTranslateExceptions(Object o) {
@@ -99,6 +93,7 @@ public class PersistenceExceptionTranslationPostProcessorTests extends TestCase
 		fail("No translation");
 	}
 
+
 	@Repository
 	public static class RepositoryWithoutInterface {
 
@@ -106,19 +101,38 @@ public class PersistenceExceptionTranslationPostProcessorTests extends TestCase
 		}
 	}
 
+
 	public interface Additional {
 
-		void additionalMethod();
+		void additionalMethod(boolean fail);
 	}
 
+
 	public static class RepositoryWithoutInterfaceAndOtherwiseAdvised extends StereotypedRepositoryInterfaceImpl
 			implements Additional {
 
 		@Override
-		public void additionalMethod() {
+		public void additionalMethod(boolean fail) {
+			if (fail) {
+				throw new PersistenceException("my failure");
+			}
 		}
 	}
 
+
+	public static class MyPersistenceExceptionTranslator implements PersistenceExceptionTranslator {
+
+
+		@Override
+		public DataAccessException translateExceptionIfPossible(RuntimeException ex) {
+			if (ex instanceof PersistenceException) {
+				return new DataAccessResourceFailureException(ex.getMessage());
+			}
+			return null;
+		}
+	}
+
+
 	@Aspect
 	public static class LogAllAspect {