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.

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<ParameterExpression<?>> 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<ParameterExpression<?>> 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());
         }
 
 

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.

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<User, Long> {
+
+        Page<User> findByFirstname(String firstname, Pageable pageable);
+    }
+}