From 3ff4f94c68489c472f4251abe9f20a0aa052e383 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 31 Mar 2014 14:35:42 +0200 Subject: [PATCH] DATAJPA-460 - Reduce implementation to core requested feature. Removed the additional deleted flag in @Query as we currently already ship with a method to manually implement delete-queries (using @Modifying and a manually defined JPQL query). Tiny optimization in DeleteExecution. Original pull request: #66. --- .../data/jpa/repository/Query.java | 8 ---- .../repository/query/AbstractJpaQuery.java | 7 +--- .../repository/query/JpaQueryExecution.java | 16 +++----- .../jpa/repository/query/JpaQueryMethod.java | 11 ------ .../repository/query/PartTreeJpaQuery.java | 9 +---- .../jpa/repository/UserRepositoryTests.java | 37 ------------------- .../jpa/repository/sample/UserRepository.java | 16 +------- 7 files changed, 9 insertions(+), 95 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/Query.java b/src/main/java/org/springframework/data/jpa/repository/Query.java index ce24d324b..849726006 100644 --- a/src/main/java/org/springframework/data/jpa/repository/Query.java +++ b/src/main/java/org/springframework/data/jpa/repository/Query.java @@ -64,12 +64,4 @@ public @interface Query { * @return */ String countName() default ""; - - /** - * Returns whether the query should delete matching entities. - * - * @since 1.6 - * @return - */ - boolean delete() default false; } diff --git a/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java index 466cb9b42..77872ab3f 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java @@ -23,7 +23,6 @@ import javax.persistence.TypedQuery; import org.springframework.data.jpa.repository.EntityGraph; import org.springframework.data.jpa.repository.query.JpaQueryExecution.CollectionExecution; -import org.springframework.data.jpa.repository.query.JpaQueryExecution.DeleteExecution; import org.springframework.data.jpa.repository.query.JpaQueryExecution.ModifyingExecution; import org.springframework.data.jpa.repository.query.JpaQueryExecution.PagedExecution; import org.springframework.data.jpa.repository.query.JpaQueryExecution.SingleEntityExecution; @@ -85,7 +84,6 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { * .lang.Object[]) */ public Object execute(Object[] parameters) { - return doExecute(getExecution(), parameters); } @@ -95,15 +93,12 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { * @return */ private Object doExecute(JpaQueryExecution execution, Object[] values) { - return execution.execute(this, values); } protected JpaQueryExecution getExecution() { - if (method.isDeleteQuery()) { - return new DeleteExecution(em); - } else if (method.isCollectionQuery()) { + if (method.isCollectionQuery()) { return new CollectionExecution(); } else if (method.isSliceQuery()) { return new SlicedExecution(method.getParameters()); diff --git a/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryExecution.java b/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryExecution.java index a9068dcb7..5e52dd5b3 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryExecution.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryExecution.java @@ -229,6 +229,7 @@ public abstract class JpaQueryExecution { * {@link Execution} removing entities matching the query. * * @author Thomas Darimont + * @author Oliver Gierke * @since 1.6 */ static class DeleteExecution extends JpaQueryExecution { @@ -244,23 +245,16 @@ public abstract class JpaQueryExecution { * @see org.springframework.data.jpa.repository.query.JpaQueryExecution#doExecute(org.springframework.data.jpa.repository.query.AbstractJpaQuery, java.lang.Object[]) */ @Override - protected Object doExecute(AbstractJpaQuery query, Object[] values) { + protected Object doExecute(AbstractJpaQuery jpaQuery, Object[] values) { - Query qry = query.createQuery(values); + Query query = jpaQuery.createQuery(values); + List resultList = query.getResultList(); - List resultList = qry.getResultList(); for (Object o : resultList) { em.remove(o); } - Object result = null; - if (query.getQueryMethod().isCollectionQuery()) { - result = resultList; - } else { - result = resultList.size(); - } - - return result; + return jpaQuery.getQueryMethod().isCollectionQuery() ? resultList : resultList.size(); } } } diff --git a/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryMethod.java b/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryMethod.java index b2f35aad6..0342e786e 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryMethod.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryMethod.java @@ -288,15 +288,4 @@ public class JpaQueryMethod extends QueryMethod { public JpaParameters getParameters() { return (JpaParameters) super.getParameters(); } - - /** - * Return {@literal true} if this backing method is a query method with the {@link Query#delete()} attribute set to - * {@literal true} else {@literal false}. - * - * @return - * @since 1.6 - */ - public boolean isDeleteQuery() { - return getAnnotationValue("delete", Boolean.class); - } } diff --git a/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java index 4942db55b..befa4abd9 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java @@ -70,7 +70,6 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { */ @Override public Query doCreateQuery(Object[] values) { - return query.createQuery(values); } @@ -81,7 +80,6 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { @Override @SuppressWarnings("unchecked") public TypedQuery doCreateCountQuery(Object[] values) { - return (TypedQuery) countQuery.createQuery(values); } @@ -91,12 +89,7 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { */ @Override protected JpaQueryExecution getExecution() { - - if (this.tree.isDelete()) { - return new DeleteExecution(em); - } - - return super.getExecution(); + return this.tree.isDelete() ? new DeleteExecution(em) : super.getExecution(); } /** 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 44ad4b494..a280c7b35 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -1298,7 +1298,6 @@ public class UserRepositoryTests { assertThat(result, hasItems(firstUser, secondUser)); } - /** * @see DATAJPA-460 */ @@ -1357,42 +1356,6 @@ public class UserRepositoryTests { assertThat(repository.deleteByLastname("dorfuaeB"), empty()); } - /** - * @see DATAJPA-460 - */ - @Test - public void deleteByUsingAnnotatedQueryShouldReturnListOfDeletedElementsWhenRetunTypeIsCollectionLike() { - - flushTestUsers(); - - List result = repository.deleteByLastnameUsingAnnotatedQuery(firstUser.getLastname()); - assertThat(result, hasItem(firstUser)); - assertThat(result, hasSize(1)); - } - - /** - * @see DATAJPA-460 - */ - @Test - public void deleteByUsingAnnotatedQueryShouldRemoveElementsMatchingDerivedQuery() { - - flushTestUsers(); - - repository.removeByLastnameUsingAnnotatedQuery(firstUser.getLastname()); - assertThat(repository.countByLastname(firstUser.getLastname()), is(0L)); - } - - /** - * @see DATAJPA-460 - */ - @Test - public void deleteByUsingAnnotatedQueryShouldReturnNumberOfDocumentsRemovedIfReturnTypeIsLong() { - - flushTestUsers(); - - assertThat(repository.removeByLastnameUsingAnnotatedQuery(firstUser.getLastname()), is(1L)); - } - private Page executeSpecWithSort(Sort sort) { flushTestUsers(); diff --git a/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java b/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java index f017b155f..efe06cb46 100644 --- a/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java +++ b/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java @@ -317,8 +317,8 @@ public interface UserRepository extends JpaRepository, JpaSpecifi * @see DATAJPA-496 */ List findByAttributesIn(Set attributes); - - /** + + /** * @see DATAJPA-460 */ Long removeByLastname(String lastname); @@ -327,16 +327,4 @@ public interface UserRepository extends JpaRepository, JpaSpecifi * @see DATAJPA-460 */ List deleteByLastname(String lastname); - - /** - * @see DATAJPA-460 - */ - @Query(value = "select u from User u where u.lastname = ?1", delete = true) - List deleteByLastnameUsingAnnotatedQuery(String lastname); - - /** - * @see DATAJPA-460 - */ - @Query(value = "select u from User u where u.lastname = ?1", delete = true) - Long removeByLastnameUsingAnnotatedQuery(String lastname); }