From df18e9173dfbf6aa97852a25db100294b72e6eb5 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 4 Sep 2013 18:33:25 +0200 Subject: [PATCH] Revised PersistenceExceptionTranslationInterceptor to lazily retrieve PersistenceExceptionTranslator beans on demand Issue: SPR-10894 --- ...stenceExceptionTranslationInterceptor.java | 62 +++++++++--------- ...xceptionTranslationPostProcessorTests.java | 64 +++++++++++-------- 2 files changed, 70 insertions(+), 56 deletions(-) diff --git a/spring-tx/src/main/java/org/springframework/dao/support/PersistenceExceptionTranslationInterceptor.java b/spring-tx/src/main/java/org/springframework/dao/support/PersistenceExceptionTranslationInterceptor.java index b486dccdfa..bca8154dc5 100644 --- a/spring-tx/src/main/java/org/springframework/dao/support/PersistenceExceptionTranslationInterceptor.java +++ b/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 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); - } - } - } - } diff --git a/spring-tx/src/test/java/org/springframework/dao/annotation/PersistenceExceptionTranslationPostProcessorTests.java b/spring-tx/src/test/java/org/springframework/dao/annotation/PersistenceExceptionTranslationPostProcessorTests.java index fb6259de93..ef2a0c2236 100644 --- a/spring-tx/src/test/java/org/springframework/dao/annotation/PersistenceExceptionTranslationPostProcessorTests.java +++ b/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 {