From ace028fa2b24c3b34e61bfc46e4d2fb18323b3d6 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Fri, 19 Apr 2013 15:23:44 +0200 Subject: [PATCH] DATAJPA-330 - Improve setup of persistence exception translation. Removed the declaration of a global PersistenceExceptionTranslationPostProcessor as the factory already applies one at the bean level selectively. Added integration tests to make sure the translation gets applied and plain @Repository classes not affected at all. --- .../config/JpaRepositoryConfigExtension.java | 11 +--- .../support/SimpleJpaRepository.java | 3 +- .../NamespaceUserRepositoryTests.java | 5 +- .../config/AbstractRepositoryConfigTests.java | 10 +++- ...RepositoriesRegistrarIntegrationTests.java | 51 ++++++++++++++++++- ...JpaRepositoryConfigExtensionUnitTests.java | 2 - 6 files changed, 63 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtension.java b/src/main/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtension.java index d19971c57..d9e711644 100644 --- a/src/main/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtension.java +++ b/src/main/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtension.java @@ -1,5 +1,5 @@ /* - * Copyright 2012 the original author or authors. + * Copyright 2012-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. @@ -50,7 +50,6 @@ import org.w3c.dom.Element; public class JpaRepositoryConfigExtension extends RepositoryConfigurationExtensionSupport { private static final Class PAB_POST_PROCESSOR = PersistenceAnnotationBeanPostProcessor.class; - private static final Class PET_POST_PROCESSOR = PersistenceExceptionTranslationPostProcessor.class; private static final String DEFAULT_TRANSACTION_MANAGER_BEAN_NAME = "transactionManager"; /* @@ -138,14 +137,6 @@ public class JpaRepositoryConfigExtension extends RepositoryConfigurationExtensi super.registerBeansForRoot(registry, configurationSource); - if (!hasBean(PET_POST_PROCESSOR, registry)) { - - AbstractBeanDefinition definition = BeanDefinitionBuilder.rootBeanDefinition(PET_POST_PROCESSOR) - .getBeanDefinition(); - - registerWithSourceAndGeneratedBeanName(registry, definition, configurationSource.getSource()); - } - if (!hasBean(PAB_POST_PROCESSOR, registry) && !registry.containsBeanDefinition(AnnotationConfigUtils.PERSISTENCE_ANNOTATION_PROCESSOR_BEAN_NAME)) { diff --git a/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java b/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java index f82690e4f..2fbfd7f17 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java +++ b/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java @@ -41,6 +41,7 @@ import org.springframework.data.jpa.domain.Specification; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.JpaSpecificationExecutor; import org.springframework.data.jpa.repository.query.QueryUtils; +import org.springframework.stereotype.Repository; import org.springframework.transaction.annotation.Transactional; import org.springframework.util.Assert; @@ -54,7 +55,7 @@ import org.springframework.util.Assert; * @param the type of the entity to handle * @param the type of the entity's identifier */ -@org.springframework.stereotype.Repository +@Repository @Transactional(readOnly = true) public class SimpleJpaRepository implements JpaRepository, JpaSpecificationExecutor { diff --git a/src/test/java/org/springframework/data/jpa/repository/NamespaceUserRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/NamespaceUserRepositoryTests.java index efc05733d..ee039a838 100644 --- a/src/test/java/org/springframework/data/jpa/repository/NamespaceUserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/NamespaceUserRepositoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2011 the original author or authors. + * Copyright 2008-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. @@ -22,7 +22,6 @@ import java.util.Map; import org.junit.Test; import org.springframework.beans.factory.ListableBeanFactory; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.dao.annotation.PersistenceExceptionTranslationPostProcessor; import org.springframework.orm.jpa.support.PersistenceAnnotationBeanPostProcessor; import org.springframework.test.context.ContextConfiguration; @@ -40,9 +39,7 @@ public class NamespaceUserRepositoryTests extends UserRepositoryTests { @Test public void registersPostProcessors() { - hasAtLeastOneBeanOfType(PersistenceAnnotationBeanPostProcessor.class); - hasAtLeastOneBeanOfType(PersistenceExceptionTranslationPostProcessor.class); } private void hasAtLeastOneBeanOfType(Class beanType) { diff --git a/src/test/java/org/springframework/data/jpa/repository/config/AbstractRepositoryConfigTests.java b/src/test/java/org/springframework/data/jpa/repository/config/AbstractRepositoryConfigTests.java index da8076fd9..d283a2b4f 100644 --- a/src/test/java/org/springframework/data/jpa/repository/config/AbstractRepositoryConfigTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/config/AbstractRepositoryConfigTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2011 the original author or authors. + * Copyright 2008-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. @@ -52,4 +52,12 @@ public abstract class AbstractRepositoryConfigTests { assertNotNull(roleRepository); assertNotNull(auditableUserRepository); } + + @Test + public void repositoriesHaveExceptionTranslationApplied() { + + JpaRepositoriesRegistrarIntegrationTests.assertExceptionTranslationActive(userRepository); + JpaRepositoriesRegistrarIntegrationTests.assertExceptionTranslationActive(roleRepository); + JpaRepositoriesRegistrarIntegrationTests.assertExceptionTranslationActive(auditableUserRepository); + } } diff --git a/src/test/java/org/springframework/data/jpa/repository/config/JpaRepositoriesRegistrarIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/config/JpaRepositoriesRegistrarIntegrationTests.java index 92cdc639c..74bcf2b04 100644 --- a/src/test/java/org/springframework/data/jpa/repository/config/JpaRepositoriesRegistrarIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/config/JpaRepositoriesRegistrarIntegrationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012 the original author or authors. + * Copyright 2012-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. @@ -15,14 +15,24 @@ */ package org.springframework.data.jpa.repository.config; +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; + +import java.util.Arrays; +import java.util.List; + import javax.persistence.EntityManagerFactory; import javax.sql.DataSource; +import org.hamcrest.Matchers; import org.junit.Test; import org.junit.runner.RunWith; +import org.springframework.aop.Advisor; +import org.springframework.aop.framework.Advised; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.dao.support.PersistenceExceptionTranslationInterceptor; import org.springframework.data.jpa.repository.sample.UserRepository; import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder; import org.springframework.orm.jpa.JpaDialect; @@ -30,9 +40,11 @@ import org.springframework.orm.jpa.JpaTransactionManager; import org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean; import org.springframework.orm.jpa.vendor.HibernateJpaDialect; import org.springframework.orm.jpa.vendor.HibernateJpaVendorAdapter; +import org.springframework.stereotype.Repository; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.util.ClassUtils; /** * Integration test for {@link JpaRepositoriesRegistrar}. @@ -46,6 +58,9 @@ public class JpaRepositoriesRegistrarIntegrationTests { @Autowired UserRepository repository; + @Autowired + SampleRepository sampleRepository; + @Configuration @EnableJpaRepositories(basePackages = "org.springframework.data.jpa.repository.sample") static class Config { @@ -74,10 +89,44 @@ public class JpaRepositoriesRegistrarIntegrationTests { public PlatformTransactionManager transactionManager() { return new JpaTransactionManager(entityManagerFactory()); } + + @Bean + public SampleRepository sampleRepository() { + return new SampleRepository(); + } } @Test public void foo() { + assertThat(repository, is(notNullValue())); + } + + /** + * @see DATAJPA-330 + */ + @Test + public void doesNotProxyPlainAtRepositoryBeans() { + + assertThat(sampleRepository, is(notNullValue())); + assertThat(ClassUtils.isCglibProxy(sampleRepository), is(false)); + + assertExceptionTranslationActive(repository); + } + + @Repository + static class SampleRepository { } + + public static void assertExceptionTranslationActive(Object repository) { + + if (repository == null) { + return; + } + + assertThat(repository, is(instanceOf(Advised.class))); + List advisors = Arrays.asList(((Advised) repository).getAdvisors()); + assertThat(advisors, Matchers. hasItem(Matchers. hasProperty("advice", + instanceOf(PersistenceExceptionTranslationInterceptor.class)))); + } } diff --git a/src/test/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtensionUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtensionUnitTests.java index ba1788004..4ce662336 100644 --- a/src/test/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtensionUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtensionUnitTests.java @@ -45,7 +45,6 @@ import org.springframework.orm.jpa.support.PersistenceAnnotationBeanPostProcesso public class JpaRepositoryConfigExtensionUnitTests { private static final String RIABPP_CLASS_NAME = "org.springframework.data.repository.core.support.RepositoryInterfaceAwareBeanPostProcessor"; - private static final String PETPP_CLASS_NAME = "org.springframework.dao.annotation.PersistenceExceptionTranslationPostProcessor"; private static final String PABPP_CLASS_NAME = "org.springframework.orm.jpa.support.PersistenceAnnotationBeanPostProcessor"; @Mock @@ -65,7 +64,6 @@ public class JpaRepositoryConfigExtensionUnitTests { Iterable names = Arrays.asList(factory.getBeanDefinitionNames()); assertThat(names, Matchers. hasItem(startsWith(PABPP_CLASS_NAME))); - assertThat(names, Matchers. hasItem(startsWith(PETPP_CLASS_NAME))); assertThat(names, Matchers. hasItem(startsWith(RIABPP_CLASS_NAME))); }