From a005f0bd6e38a965207a43e693a78bea608f05f5 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 9 Feb 2011 13:53:07 +0100 Subject: [PATCH] Forward ported fix of Hades bug #421. Moved transaction configuration into JpaRepository interface and redeclared methods of Repository and PagingAndSortingRepository. This is unfortunately necessary as otherwise the custom transaction configuration inside a user's repository interface (e.g. redefining transaction configuration in findAll()) would not be considered as AbstractFallbackTransactionAttributeSource prefers implementation configuration as this is usually more specific (except in our case). This unfortunately requires the redeclaration of the methods. --- .../data/jpa/repository/JpaRepository.java | 118 +++++++++++++++++- .../support/SimpleJpaRepository.java | 10 -- .../CustomRepositoryFactoryConfigTests.java | 35 ++++++ .../custom/UserCustomExtendedRepository.java | 17 +++ .../namespace-customfactory-context.xml | 1 + 5 files changed, 170 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/JpaRepository.java b/src/main/java/org/springframework/data/jpa/repository/JpaRepository.java index 8bfd19b4f..d1bd120fd 100644 --- a/src/main/java/org/springframework/data/jpa/repository/JpaRepository.java +++ b/src/main/java/org/springframework/data/jpa/repository/JpaRepository.java @@ -22,19 +22,132 @@ import javax.persistence.EntityManager; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.Sort; import org.springframework.data.jpa.domain.Specification; import org.springframework.data.repository.PagingAndSortingRepository; import org.springframework.data.repository.Repository; +import org.springframework.transaction.annotation.Transactional; /** - * JPA specific extension of {@link Repository}. + * JPA specific extension of {@link Repository}. Redeclares methods from + * {@link Repository} and {@link PagingAndSortingRepository} to apply + * transaction configuration to those. We need to do this to allow specific + * extensions of the interface to override transaction configuration. If we'd + * annotated the implementation this configuration would always enjoy precedence + * over the configuration applied on an interface. * * @author Oliver Gierke */ +@Transactional(readOnly = true) public interface JpaRepository extends PagingAndSortingRepository { + /* + * (non-Javadoc) + * + * @see + * org.springframework.data.repository.Repository#save(java.lang.Object) + */ + @Transactional + T save(T entity); + + + /* + * (non-Javadoc) + * + * @see + * org.springframework.data.repository.Repository#save(java.lang.Iterable) + */ + @Transactional + List save(Iterable entities); + + + /* + * (non-Javadoc) + * + * @see + * org.springframework.data.repository.Repository#findById(java.io.Serializable + * ) + */ + T findById(ID id); + + + /* + * (non-Javadoc) + * + * @see + * org.springframework.data.repository.Repository#exists(java.io.Serializable + * ) + */ + boolean exists(ID id); + + + /* + * (non-Javadoc) + * + * @see org.springframework.data.repository.Repository#findAll() + */ + List findAll(); + + + /* + * (non-Javadoc) + * + * @see org.springframework.data.repository.Repository#count() + */ + Long count(); + + + /* + * (non-Javadoc) + * + * @see + * org.springframework.data.repository.Repository#delete(java.lang.Object) + */ + @Transactional + void delete(T entity); + + + /* + * (non-Javadoc) + * + * @see + * org.springframework.data.repository.Repository#delete(java.lang.Iterable) + */ + @Transactional + void delete(Iterable entities); + + + /* + * (non-Javadoc) + * + * @see org.springframework.data.repository.Repository#deleteAll() + */ + @Transactional + void deleteAll(); + + + /* + * (non-Javadoc) + * + * @see + * org.springframework.data.repository.PagingAndSortingRepository#findAll + * (org.springframework.data.domain.Sort) + */ + List findAll(Sort sort); + + + /* + * (non-Javadoc) + * + * @see + * org.springframework.data.repository.PagingAndSortingRepository#findAll + * (org.springframework.data.domain.Pageable) + */ + Page findAll(Pageable pageable); + + /** * Returns a single entity matching the given {@link Specification}. * @@ -67,6 +180,7 @@ public interface JpaRepository extends /** * Flushes all pending changes to the database. */ + @Transactional void flush(); @@ -76,6 +190,7 @@ public interface JpaRepository extends * @param entity * @return the saved entity */ + @Transactional T saveAndFlush(T entity); @@ -86,5 +201,6 @@ public interface JpaRepository extends * * @param entities */ + @Transactional void deleteInBatch(Iterable entities); } 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 50f633658..06a3d6b00 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 @@ -35,7 +35,6 @@ import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.data.jpa.domain.Specification; import org.springframework.data.repository.Repository; -import org.springframework.transaction.annotation.Transactional; import org.springframework.util.Assert; @@ -49,7 +48,6 @@ import org.springframework.util.Assert; * @param the type of the entity's identifier */ @org.springframework.stereotype.Repository -@Transactional public class SimpleJpaRepository extends JpaRepositorySupport { @@ -174,7 +172,6 @@ public class SimpleJpaRepository extends * org.springframework.data.repository.Repository#readById(java.io.Serializable * ) */ - @Transactional(readOnly = true) public T findById(ID id) { Assert.notNull(id, "The given id must not be null!"); @@ -189,7 +186,6 @@ public class SimpleJpaRepository extends * org.springframework.data.repository.Repository#exists(java.io.Serializable * ) */ - @Transactional(readOnly = true) public boolean exists(ID id) { Assert.notNull(id, "The given id must not be null!"); @@ -202,7 +198,6 @@ public class SimpleJpaRepository extends * * @see org.springframework.data.repository.Repository#readAll() */ - @Transactional(readOnly = true) public List findAll() { return getQuery(null, (Sort) null).getResultList(); @@ -216,7 +211,6 @@ public class SimpleJpaRepository extends * org.springframework.data.repository.Repository#readAll(org.springframework * .data.domain.Sort) */ - @Transactional(readOnly = true) public List findAll(Sort sort) { return getQuery(null, sort).getResultList(); @@ -229,7 +223,6 @@ public class SimpleJpaRepository extends * @see org.springframework.data.repository.Repository#readAll(org. * springframework.data.domain.Pageable) */ - @Transactional(readOnly = true) public Page findAll(Pageable pageable) { if (null == pageable) { @@ -262,7 +255,6 @@ public class SimpleJpaRepository extends * @see org.springframework.data.jpa.repository.JpaRepository#readAll(org. * springframework.data.jpa.domain.Specification) */ - @Transactional(readOnly = true) public List findAll(Specification spec) { return getQuery(spec, (Sort) null).getResultList(); @@ -276,7 +268,6 @@ public class SimpleJpaRepository extends * springframework.data.jpa.domain.Specification, * org.springframework.data.domain.Pageable) */ - @Transactional(readOnly = true) public Page findAll(Specification spec, Pageable pageable) { TypedQuery query = getQuery(spec, pageable); @@ -291,7 +282,6 @@ public class SimpleJpaRepository extends * * @see org.springframework.data.repository.Repository#count() */ - @Transactional(readOnly = true) public Long count() { return em.createQuery(getCountQueryString(), Long.class) diff --git a/src/test/java/org/springframework/data/jpa/repository/config/CustomRepositoryFactoryConfigTests.java b/src/test/java/org/springframework/data/jpa/repository/config/CustomRepositoryFactoryConfigTests.java index 1b9791911..3d023d157 100644 --- a/src/test/java/org/springframework/data/jpa/repository/config/CustomRepositoryFactoryConfigTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/config/CustomRepositoryFactoryConfigTests.java @@ -15,10 +15,15 @@ */ package org.springframework.data.jpa.repository.config; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; + +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.jpa.repository.custom.UserCustomExtendedRepository; +import org.springframework.data.jpa.repository.support.TransactionalRepositoryTests.DelegatingTransactionManager; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.util.Assert; @@ -43,6 +48,16 @@ public class CustomRepositoryFactoryConfigTests { @Autowired(required = false) UserCustomExtendedRepository userRepository; + @Autowired + DelegatingTransactionManager transactionManager; + + + @Before + public void setup() { + + transactionManager.resetCount(); + } + @Test(expected = UnsupportedOperationException.class) public void testCustomFactoryUsed() { @@ -50,4 +65,24 @@ public class CustomRepositoryFactoryConfigTests { Assert.notNull(userRepository); userRepository.customMethod(1); } + + + @Test + public void reconfiguresTransactionalMethodWithoutGenericParameter() { + + userRepository.findAll(); + + assertFalse(transactionManager.getDefinition().isReadOnly()); + assertThat(transactionManager.getDefinition().getTimeout(), is(10)); + } + + + @Test + public void reconfiguresTransactionalMethodWithGenericParameter() { + + userRepository.findById(1); + + assertFalse(transactionManager.getDefinition().isReadOnly()); + assertThat(transactionManager.getDefinition().getTimeout(), is(10)); + } } diff --git a/src/test/java/org/springframework/data/jpa/repository/custom/UserCustomExtendedRepository.java b/src/test/java/org/springframework/data/jpa/repository/custom/UserCustomExtendedRepository.java index 43a3e44ec..67ec1250d 100644 --- a/src/test/java/org/springframework/data/jpa/repository/custom/UserCustomExtendedRepository.java +++ b/src/test/java/org/springframework/data/jpa/repository/custom/UserCustomExtendedRepository.java @@ -15,7 +15,10 @@ */ package org.springframework.data.jpa.repository.custom; +import java.util.List; + import org.springframework.data.jpa.domain.sample.User; +import org.springframework.transaction.annotation.Transactional; /** @@ -27,4 +30,18 @@ import org.springframework.data.jpa.domain.sample.User; public interface UserCustomExtendedRepository extends CustomGenericRepository { + /** + * Sample method to test reconfiguring transactions on CRUD methods in + * combination with custom factory. + * + * @see #421 + */ + + @Transactional(readOnly = false, timeout = 10) + List findAll(); + + + @Transactional(readOnly = false, timeout = 10) + User findById(Integer id); + } \ No newline at end of file diff --git a/src/test/resources/config/namespace-customfactory-context.xml b/src/test/resources/config/namespace-customfactory-context.xml index d5d3d78b7..e621ee2cb 100644 --- a/src/test/resources/config/namespace-customfactory-context.xml +++ b/src/test/resources/config/namespace-customfactory-context.xml @@ -7,6 +7,7 @@ http://www.springframework.org/schema/data/jpa http://www.springframework.org/schema/data/jpa/spring-jpa.xsd"> +