From fcb4992983b221f53e7edc1fa2d9f9d49dc6d039 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Sun, 21 Aug 2011 12:23:16 +0200 Subject: [PATCH] DATAJPA-90 - Refactored PartTreeJpaQuery to create new JpaQueryCreator on every query recreation. Fixed potential concurrency problem for PartTree based query creation. JpaQueryCreator is stateful as we create a list of ParameterExpressions and iterate over them. So we have to re-instantiate the JpaQueryCreator to not create an exception on the second attempt. We now eagerly create CriteriaQuery and it's ParameterExpressions in QueryPreparer's constructor. Lazily doing so could let to exceptions accessing the ParameterExpressions in race conditions. --- .../repository/query/PartTreeJpaQuery.java | 94 +++++++++---------- .../support/PersistenceProvider.java | 2 +- .../PartTreeJpaQueryIntegrationTests.java | 74 +++++++++++++++ 3 files changed, 119 insertions(+), 51 deletions(-) create mode 100644 src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java 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 4993c47d4..137c2578a 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 @@ -27,7 +27,6 @@ import org.springframework.data.domain.Sort; import org.springframework.data.repository.query.Parameters; import org.springframework.data.repository.query.ParametersParameterAccessor; import org.springframework.data.repository.query.parser.PartTree; -import org.springframework.util.Assert; /** @@ -59,8 +58,10 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { this.tree = new PartTree(method.getName(), domainClass); this.parameters = method.getParameters(); - this.query = new QueryPreparer(tree, domainClass, parameters); - this.countQuery = new CountQueryPreparer(tree, domainClass, parameters); + this.query = + new QueryPreparer(parameters.potentiallySortsDynamically()); + this.countQuery = + new CountQueryPreparer(parameters.potentiallySortsDynamically()); } @@ -99,37 +100,16 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { */ private class QueryPreparer { - private CriteriaQuery query; - private final JpaQueryCreator creator; + private final CriteriaQuery query; + private final List> expressions; - /** - * Creates a new {@link QueryPreparer} from the given {@link PartTree}, - * domain class and {@link Parameters}. - * - * @param tree - * @param domainClass - * @param parameters - */ - public QueryPreparer(PartTree tree, Class domainClass, - Parameters parameters) { + public QueryPreparer(boolean recreateQueries) { - this(new JpaQueryCreator(tree, domainClass, parameters, - getEntityManager())); - } - - - /** - * Creates a new {@link QueryPreparer} from the given - * {@link JpaQueryCreator}. - * - * @param creator must not be {@literl null}. - */ - protected QueryPreparer(JpaQueryCreator creator) { - - Assert.notNull(creator); - this.creator = creator; - this.query = null; + JpaQueryCreator creator = createCreator(); + this.query = recreateQueries ? null : creator.createQuery(); + this.expressions = + recreateQueries ? null : creator.getParameterExpressions(); } @@ -141,14 +121,25 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { */ public Query createQuery(Object[] values) { - if (parameters.potentiallySortsDynamically() || query == null) { - query = creator.createQuery(getDynamicSort(values)); + CriteriaQuery criteriaQuery = query; + List> expressions = this.expressions; + + if (query == null) { + JpaQueryCreator creator = createCreator(); + criteriaQuery = creator.createQuery(getDynamicSort(values)); + expressions = creator.getParameterExpressions(); } - TypedQuery jpaQuery = getEntityManager().createQuery(query); - return invokeBinding( - getBinder(values, creator.getParameterExpressions()), - jpaQuery); + TypedQuery jpaQuery = + getEntityManager().createQuery(criteriaQuery); + return invokeBinding(getBinder(values, expressions), jpaQuery); + } + + + protected JpaQueryCreator createCreator() { + + return new JpaQueryCreator(tree, domainClass, parameters, + getEntityManager()); } @@ -188,20 +179,23 @@ public class PartTreeJpaQuery extends AbstractJpaQuery { */ private class CountQueryPreparer extends QueryPreparer { - /** - * Creates a new {@link CountQueryPreparer} from the given - * {@link PartTree}, domain class and {@link Parameters}. Will use a - * {@link JpaCountQueryCreator} to create the query. - * - * @param tree - * @param domainClass - * @param parameters - */ - public CountQueryPreparer(PartTree tree, Class domainClass, - Parameters parameters) { + public CountQueryPreparer(boolean recreateQueries) { - super(new JpaCountQueryCreator(tree, domainClass, parameters, - getEntityManager())); + super(recreateQueries); + } + + + /* + * (non-Javadoc) + * + * @see org.springframework.data.jpa.repository.query.PartTreeJpaQuery. + * QueryPreparer#createCreator() + */ + @Override + protected JpaQueryCreator createCreator() { + + return new JpaCountQueryCreator(tree, domainClass, parameters, + getEntityManager()); } diff --git a/src/main/java/org/springframework/data/jpa/repository/support/PersistenceProvider.java b/src/main/java/org/springframework/data/jpa/repository/support/PersistenceProvider.java index 5ec4015c1..9cf9c55d7 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/PersistenceProvider.java +++ b/src/main/java/org/springframework/data/jpa/repository/support/PersistenceProvider.java @@ -31,7 +31,7 @@ import org.springframework.data.jpa.repository.query.QueryExtractor; * * @author Oliver Gierke */ -enum PersistenceProvider implements QueryExtractor { +public enum PersistenceProvider implements QueryExtractor { /** * Hibernate persistence provider. diff --git a/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java new file mode 100644 index 000000000..3524d48f0 --- /dev/null +++ b/src/test/java/org/springframework/data/jpa/repository/query/PartTreeJpaQueryIntegrationTests.java @@ -0,0 +1,74 @@ +/* + * Copyright 2011 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.jpa.repository.query; + +import java.lang.reflect.Method; + +import javax.persistence.EntityManager; +import javax.persistence.PersistenceContext; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.PageRequest; +import org.springframework.data.domain.Pageable; +import org.springframework.data.jpa.domain.sample.User; +import org.springframework.data.jpa.repository.support.PersistenceProvider; +import org.springframework.data.repository.Repository; +import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + + +/** + * Integration tests for {@link PartTreeJpaQuery}. + * + * @author Oliver Gierke + */ +@RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration("classpath:infrastructure.xml") +public class PartTreeJpaQueryIntegrationTests { + + @PersistenceContext + EntityManager entityManager; + + + /** + * @see DATADOC-90 + * @throws Exception + */ + @Test + public void test() throws Exception { + + Method method = + UserRepository.class.getMethod("findByFirstname", String.class, + Pageable.class); + JpaQueryMethod queryMethod = + new JpaQueryMethod(method, new DefaultRepositoryMetadata( + UserRepository.class), + PersistenceProvider.fromEntityManager(entityManager)); + PartTreeJpaQuery jpaQuery = + new PartTreeJpaQuery(queryMethod, entityManager); + + jpaQuery.createQuery(new Object[] { "Matthews", new PageRequest(0, 1) }); + jpaQuery.createQuery(new Object[] { "Matthews", new PageRequest(0, 1) }); + } + + interface UserRepository extends Repository { + + Page findByFirstname(String firstname, Pageable pageable); + } +}