From add1336ed1a472a74a1afa73d828669fc40de712 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Fri, 2 Dec 2011 13:45:07 +0100 Subject: [PATCH] DATACMNS-91 - Enforce parameter handling defined by CrudRepository. Reject null values for id and entity values. Polished JavaDoc in terms of parameter specification. --- .../support/JpaEntityInformationSupport.java | 10 +- .../support/SimpleJpaRepository.java | 116 ++++++------------ .../jpa/repository/UserRepositoryTests.java | 6 - 3 files changed, 44 insertions(+), 88 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/support/JpaEntityInformationSupport.java b/src/main/java/org/springframework/data/jpa/repository/support/JpaEntityInformationSupport.java index 2d86ee422..4eb9542a1 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/JpaEntityInformationSupport.java +++ b/src/main/java/org/springframework/data/jpa/repository/support/JpaEntityInformationSupport.java @@ -23,6 +23,7 @@ import javax.persistence.metamodel.Metamodel; import org.springframework.data.domain.Persistable; import org.springframework.data.repository.core.support.AbstractEntityInformation; +import org.springframework.util.Assert; import org.springframework.util.StringUtils; /** @@ -36,7 +37,7 @@ public abstract class JpaEntityInformationSupport ex /** * Creates a new {@link JpaEntityInformationSupport} with the given domain class. * - * @param domainClass + * @param domainClass must not be {@literal null}. */ public JpaEntityInformationSupport(Class domainClass) { @@ -46,13 +47,16 @@ public abstract class JpaEntityInformationSupport ex /** * Creates a {@link JpaEntityInformation} for the given domain class and {@link EntityManager}. * - * @param domainClass - * @param em + * @param domainClass must not be {@literal null}. + * @param em must not be {@literal null}. * @return */ @SuppressWarnings({ "rawtypes", "unchecked" }) public static JpaEntityInformation getMetadata(Class domainClass, EntityManager em) { + Assert.notNull(domainClass); + Assert.notNull(em); + Metamodel metamodel = em.getMetamodel(); if (Persistable.class.isAssignableFrom(domainClass)) { 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 82b761d48..9bb84de35 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 @@ -61,13 +61,14 @@ public class SimpleJpaRepository implements JpaRepos /** * Creates a new {@link SimpleJpaRepository} to manage objects of the given {@link JpaEntityInformation}. * - * @param entityInformation - * @param entityManager + * @param entityInformation must not be {@literal null}. + * @param entityManager must not be {@literal null}. */ public SimpleJpaRepository(JpaEntityInformation entityInformation, EntityManager entityManager) { Assert.notNull(entityInformation); Assert.notNull(entityManager); + this.entityInformation = entityInformation; this.em = entityManager; this.provider = PersistenceProvider.fromEntityManager(entityManager); @@ -76,21 +77,18 @@ public class SimpleJpaRepository implements JpaRepos /** * Creates a new {@link SimpleJpaRepository} to manage objects of the given domain type. * - * @param domainClass - * @param em + * @param domainClass must not be {@literal null}. + * @param em must not be {@literal null}. */ public SimpleJpaRepository(Class domainClass, EntityManager em) { - this(JpaEntityInformationSupport.getMetadata(domainClass, em), em); } private Class getDomainClass() { - return entityInformation.getJavaType(); } private String getDeleteAllQueryString() { - return getQueryString(DELETE_ALL_QUERY_STRING, entityInformation.getEntityName()); } @@ -102,41 +100,34 @@ public class SimpleJpaRepository implements JpaRepos /* * (non-Javadoc) - * - * @see - * org.springframework.data.jpa.repository.JpaRepository#delete(java.io. - * Serializable) + * @see org.springframework.data.repository.CrudRepository#delete(java.io.Serializable) */ @Transactional public void delete(ID id) { + Assert.notNull(id, "The given id must not be null!"); delete(findOne(id)); } /* * (non-Javadoc) - * - * @see - * org.springframework.data.repository.Repository#delete(java.lang.Object) + * @see org.springframework.data.repository.CrudRepository#delete(java.lang.Object) */ @Transactional public void delete(T entity) { + Assert.notNull(entity, "The entity must not be null!"); em.remove(em.contains(entity) ? entity : em.merge(entity)); } /* * (non-Javadoc) - * - * @see - * org.springframework.data.repository.Repository#delete(java.lang.Iterable) + * @see org.springframework.data.repository.CrudRepository#delete(java.lang.Iterable) */ @Transactional public void delete(Iterable entities) { - if (entities == null) { - return; - } + Assert.notNull(entities, "The given Iterable of entities not be null!"); for (T entity : entities) { delete(entity); @@ -145,15 +136,14 @@ public class SimpleJpaRepository implements JpaRepos /* * (non-Javadoc) - * - * @see - * org.springframework.data.jpa.repository.JpaRepository#deleteInBatch(java - * .lang.Iterable) + * @see org.springframework.data.jpa.repository.JpaRepository#deleteInBatch(java.lang.Iterable) */ @Transactional public void deleteInBatch(Iterable entities) { - if (null == entities || !entities.iterator().hasNext()) { + Assert.notNull(entities, "The given Iterable of entities not be null!"); + + if (!entities.iterator().hasNext()) { return; } @@ -163,12 +153,10 @@ public class SimpleJpaRepository implements JpaRepos /* * (non-Javadoc) - * * @see org.springframework.data.repository.Repository#deleteAll() */ @Transactional public void deleteAll() { - em.createQuery(getDeleteAllQueryString()).executeUpdate(); } @@ -191,6 +179,8 @@ public class SimpleJpaRepository implements JpaRepos */ public boolean exists(ID id) { + Assert.notNull(id, "The given id must not be null!"); + String placeholder = provider.getCountQueryPlaceholder(); String entityName = entityInformation.getEntityName(); String idAttributeName = entityInformation.getIdAttribute().getName(); @@ -208,27 +198,20 @@ public class SimpleJpaRepository implements JpaRepos * @see org.springframework.data.jpa.repository.JpaRepository#findAll() */ public List findAll() { - return getQuery(null, (Sort) null).getResultList(); } /* * (non-Javadoc) - * - * @see - * org.springframework.data.repository.Repository#readAll(org.springframework - * .data.domain.Sort) + * @see org.springframework.data.jpa.repository.JpaRepository#findAll(org.springframework.data.domain.Sort) */ public List findAll(Sort sort) { - return getQuery(null, sort).getResultList(); } /* * (non-Javadoc) - * - * @see org.springframework.data.repository.Repository#readAll(org. - * springframework.data.domain.Pageable) + * @see org.springframework.data.repository.PagingAndSortingRepository#findAll(org.springframework.data.domain.Pageable) */ public Page findAll(Pageable pageable) { @@ -241,9 +224,7 @@ public class SimpleJpaRepository implements JpaRepos /* * (non-Javadoc) - * - * @see org.springframework.data.jpa.repository.JpaRepository#findOneBy(org. - * springframework.data.jpa.domain.Specification) + * @see org.springframework.data.jpa.repository.JpaSpecificationExecutor#findOne(org.springframework.data.jpa.domain.Specification) */ public T findOne(Specification spec) { @@ -256,36 +237,25 @@ public class SimpleJpaRepository implements JpaRepos /* * (non-Javadoc) - * - * @see org.springframework.data.jpa.repository.JpaRepository#readAll(org. - * springframework.data.jpa.domain.Specification) + * @see org.springframework.data.jpa.repository.JpaSpecificationExecutor#findAll(org.springframework.data.jpa.domain.Specification) */ public List findAll(Specification spec) { - return getQuery(spec, (Sort) null).getResultList(); } /* * (non-Javadoc) - * - * @see org.springframework.data.jpa.repository.JpaRepository#readAll(org. - * springframework.data.jpa.domain.Specification, - * org.springframework.data.domain.Pageable) + * @see org.springframework.data.jpa.repository.JpaSpecificationExecutor#findAll(org.springframework.data.jpa.domain.Specification, org.springframework.data.domain.Pageable) */ public Page findAll(Specification spec, Pageable pageable) { TypedQuery query = getQuery(spec, pageable); - return pageable == null ? new PageImpl(query.getResultList()) : readPage(query, pageable, spec); } /* * (non-Javadoc) - * - * @see - * org.springframework.data.jpa.repository.JpaSpecificationExecutor#findAll - * (org.springframework.data.jpa.domain.Specification, - * org.springframework.data.domain.Sort) + * @see org.springframework.data.jpa.repository.JpaSpecificationExecutor#findAll(org.springframework.data.jpa.domain.Specification, org.springframework.data.domain.Sort) */ public List findAll(Specification spec, Sort sort) { @@ -294,8 +264,7 @@ public class SimpleJpaRepository implements JpaRepos /* * (non-Javadoc) - * - * @see org.springframework.data.repository.Repository#count() + * @see org.springframework.data.repository.CrudRepository#count() */ public long count() { return em.createQuery(getCountQueryString(), Long.class).getSingleResult(); @@ -303,10 +272,7 @@ public class SimpleJpaRepository implements JpaRepos /* * (non-Javadoc) - * - * @see - * org.springframework.data.jpa.repository.JpaSpecificationExecutor#count - * (org.springframework.data.jpa.domain.Specification) + * @see org.springframework.data.jpa.repository.JpaSpecificationExecutor#count(org.springframework.data.jpa.domain.Specification) */ public long count(Specification spec) { @@ -315,9 +281,7 @@ public class SimpleJpaRepository implements JpaRepos /* * (non-Javadoc) - * - * @see - * org.springframework.data.repository.Repository#save(java.lang.Object) + * @see org.springframework.data.repository.CrudRepository#save(java.lang.Object) */ @Transactional public T save(T entity) { @@ -332,10 +296,7 @@ public class SimpleJpaRepository implements JpaRepos /* * (non-Javadoc) - * - * @see - * org.springframework.data.jpa.repository.JpaRepository#saveAndFlush(java - * .lang.Object) + * @see org.springframework.data.jpa.repository.JpaRepository#saveAndFlush(java.lang.Object) */ @Transactional public T saveAndFlush(T entity) { @@ -348,9 +309,7 @@ public class SimpleJpaRepository implements JpaRepos /* * (non-Javadoc) - * - * @see - * org.springframework.data.repository.Repository#save(java.lang.Iterable) + * @see org.springframework.data.jpa.repository.JpaRepository#save(java.lang.Iterable) */ @Transactional public List save(Iterable entities) { @@ -370,7 +329,6 @@ public class SimpleJpaRepository implements JpaRepos /* * (non-Javadoc) - * * @see org.springframework.data.jpa.repository.JpaRepository#flush() */ @Transactional @@ -383,9 +341,9 @@ public class SimpleJpaRepository implements JpaRepos * Reads the given {@link TypedQuery} into a {@link Page} applying the given {@link Pageable} and * {@link Specification}. * - * @param query - * @param spec - * @param pageable + * @param query must not be {@literal null}. + * @param spec can be {@literal null}. + * @param pageable can be {@literal null}. * @return */ private Page readPage(TypedQuery query, Pageable pageable, Specification spec) { @@ -401,8 +359,8 @@ public class SimpleJpaRepository implements JpaRepos /** * Creates a new {@link TypedQuery} from the given {@link Specification}. * - * @param spec can be {@literal null} - * @param pageable can be {@literal null} + * @param spec can be {@literal null}. + * @param pageable can be {@literal null}. * @return */ private TypedQuery getQuery(Specification spec, Pageable pageable) { @@ -423,8 +381,8 @@ public class SimpleJpaRepository implements JpaRepos /** * Creates a {@link TypedQuery} for the given {@link Specification} and {@link Sort}. * - * @param spec - * @param sort + * @param spec can be {@literal null}. + * @param sort can be {@literal null}. * @return */ private TypedQuery getQuery(Specification spec, Sort sort) { @@ -462,8 +420,8 @@ public class SimpleJpaRepository implements JpaRepos /** * Applies the given {@link Specification} to the given {@link CriteriaQuery}. * - * @param spec can be {@literal null} - * @param query + * @param spec can be {@literal null}. + * @param query must not be {@literal null}. * @return */ private Root applySpecificationToCriteria(Specification spec, CriteriaQuery query) { diff --git a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java index 1aaa76bc3..a848cacc1 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -224,12 +224,6 @@ public class UserRepositoryTests { assertDeleteCallDoesNotDeleteAnything(new ArrayList()); } - @Test - public void deleteWithNullDoesNotDeleteAnything() throws Exception { - - assertDeleteCallDoesNotDeleteAnything(null); - } - @Test public void executesManipulatingQuery() throws Exception {