From f466744acd93d3778c298cb2241bab3f35e0dc56 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 15 May 2012 17:38:24 +0200 Subject: [PATCH] DATAJPA-207 - Fixed query creation for projections on native queries. When executing a projecting query through a native query we must not use em.createNativeQuery(String, Class) as this expects an entity type as type parameter. We now use the newly introduced isQueryMethodForEntity() on JpaQueryMethod to determine whether the query is actually projecting and rather use the plain em.createNativeQuery(String). Fixed dependency configuration for Hamcrest and JUnit (upgrade to JUnit 4.10, depending on junit-dep). --- pom.xml | 20 +++---- .../jpa/repository/query/JpaQueryMethod.java | 9 ++++ .../jpa/repository/query/SimpleJpaQuery.java | 8 +-- .../CustomHsqlHibernateJpaVendorAdaptor.java | 53 +++++++++++++++++++ .../jpa/repository/UserRepositoryTests.java | 14 +++++ .../AuditingBeanDefinitionParserTests.java | 2 +- .../query/JpaQueryMethodUnitTests.java | 24 ++++++++- .../jpa/repository/sample/UserRepository.java | 2 + ...odelEntityInformationIntegrationTests.java | 2 +- src/test/resources/hibernate.xml | 3 +- 10 files changed, 118 insertions(+), 19 deletions(-) create mode 100644 src/test/java/org/springframework/data/jpa/repository/CustomHsqlHibernateJpaVendorAdaptor.java diff --git a/pom.xml b/pom.xml index 7ed69e9e4..49913b0fa 100644 --- a/pom.xml +++ b/pom.xml @@ -57,11 +57,11 @@ [${spring.version.30}, ${spring.version.40}) 1.3.0.BUILD-SNAPSHOT 3.6.9.Final - 2.1.1 + 2.2.0 2.3.2 1.6.12 2.5.0 - 4.8.1 + 4.10 2.0.0 1.6.1 1.0 @@ -338,9 +338,16 @@ test + + org.hamcrest + hamcrest-library + 1.2.1 + test + + junit - junit + junit-dep ${junit.version} test @@ -352,13 +359,6 @@ test - - org.hamcrest - hamcrest-all - 1.1 - test - - org.hsqldb hsqldb 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 243875b6f..429146cff 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 @@ -227,6 +227,15 @@ public class JpaQueryMethod extends QueryMethod { return (Boolean) AnnotationUtils.getValue(method.getAnnotation(Modifying.class), "clearAutomatically"); } + /** + * Returns whether the query method will actually return entities. + * + * @return + */ + boolean isQueryMethodForEntity() { + return getDomainClass().isAssignableFrom(getReturnedObjectType()); + } + /** * Returns the {@link Query} annotation's attribute casted to the given type or default value if no annotation * available. diff --git a/src/main/java/org/springframework/data/jpa/repository/query/SimpleJpaQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/SimpleJpaQuery.java index eec317190..36502e838 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/SimpleJpaQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/SimpleJpaQuery.java @@ -85,15 +85,15 @@ final class SimpleJpaQuery extends AbstractJpaQuery { ParameterAccessor accessor = new ParametersParameterAccessor(method.getParameters(), values); String sortedQueryString = QueryUtils.applySorting(queryString, accessor.getSort(), alias); + EntityManager em = getEntityManager(); Query query = null; if (method.isNativeQuery()) { - query = method.isModifyingQuery() ? getEntityManager().createNativeQuery(sortedQueryString) : getEntityManager() - .createNativeQuery(sortedQueryString, method.getReturnedObjectType()); + query = method.isQueryMethodForEntity() ? em.createNativeQuery(sortedQueryString, method.getReturnedObjectType()) + : em.createNativeQuery(sortedQueryString); } else { - query = method.isModifyingQuery() ? getEntityManager().createQuery(sortedQueryString) : getEntityManager() - .createQuery(sortedQueryString); + query = em.createQuery(sortedQueryString); } return createBinder(values).bindAndPrepare(query); diff --git a/src/test/java/org/springframework/data/jpa/repository/CustomHsqlHibernateJpaVendorAdaptor.java b/src/test/java/org/springframework/data/jpa/repository/CustomHsqlHibernateJpaVendorAdaptor.java new file mode 100644 index 000000000..c634d1fba --- /dev/null +++ b/src/test/java/org/springframework/data/jpa/repository/CustomHsqlHibernateJpaVendorAdaptor.java @@ -0,0 +1,53 @@ +/* + * Copyright 2012 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; + +import java.sql.Types; + +import org.hibernate.dialect.HSQLDialect; +import org.springframework.orm.jpa.vendor.Database; +import org.springframework.orm.jpa.vendor.HibernateJpaVendorAdapter; + +/** + * Fix for missing type declarations for HSQL. + * + * @see http://www.codesmell.org/blog/2008/12/hibernate-hsql-native-queries-and-booleans/ + * @author Oliver Gierke + */ +public class CustomHsqlHibernateJpaVendorAdaptor extends HibernateJpaVendorAdapter { + + /* + * (non-Javadoc) + * @see org.springframework.orm.jpa.vendor.HibernateJpaVendorAdapter#determineDatabaseDialectClass(org.springframework.orm.jpa.vendor.Database) + */ + @Override + protected Class determineDatabaseDialectClass(Database database) { + + if (Database.HSQL.equals(database)) { + return CustomHsqlDialect.class; + } + + return super.determineDatabaseDialectClass(database); + } + + public static class CustomHsqlDialect extends HSQLDialect { + + public CustomHsqlDialect() { + registerColumnType(Types.BOOLEAN, "boolean"); + registerHibernateType(Types.BOOLEAN, "boolean"); + } + } +} 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 d36b4946e..9febfafbf 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -836,6 +836,20 @@ public class UserRepositoryTests { assertThat(page.getContent(), hasItems(firstUser, secondUser, thirdUser)); } + /** + * @see DATAJPA-207 + */ + @Test + public void executesNativeQueryForNonEntitiesCorrectly() { + + flushTestUsers(); + + List result = repository.findOnesByNativeQuery(); + + assertThat(result.size(), is(3)); + assertThat(result, hasItem(1)); + } + protected void flushTestUsers() { firstUser = repository.save(firstUser); diff --git a/src/test/java/org/springframework/data/jpa/repository/config/AuditingBeanDefinitionParserTests.java b/src/test/java/org/springframework/data/jpa/repository/config/AuditingBeanDefinitionParserTests.java index 1f0912d4c..c218431e5 100644 --- a/src/test/java/org/springframework/data/jpa/repository/config/AuditingBeanDefinitionParserTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/config/AuditingBeanDefinitionParserTests.java @@ -57,7 +57,7 @@ public class AuditingBeanDefinitionParserTests { PropertyValue value = definition.getPropertyValues().getPropertyValue("dateTimeProvider"); assertThat(value, is(notNullValue())); - assertThat(value.getValue(), is(RuntimeBeanReference.class)); + assertThat(value.getValue(), is(instanceOf(RuntimeBeanReference.class))); assertThat(((RuntimeBeanReference) value.getValue()).getBeanName(), is("dateTimeProvider")); BeanFactory factory = loadFactoryFrom(location); diff --git a/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryMethodUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryMethodUnitTests.java index abaf543e2..d8e2933a3 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryMethodUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryMethodUnitTests.java @@ -60,7 +60,7 @@ public class JpaQueryMethodUnitTests { RepositoryMetadata metadata; Method repositoryMethod, invalidReturnType, pageableAndSort, pageableTwice, sortableTwice, modifyingMethod, - nativeQuery, namedQuery, findWithLockMethod, invalidNamedParameter; + nativeQuery, namedQuery, findWithLockMethod, invalidNamedParameter, findsProjections, findsProjection; /** * @throws Exception @@ -82,6 +82,9 @@ public class JpaQueryMethodUnitTests { findWithLockMethod = ValidRepository.class.getMethod("findOneLocked", Integer.class); invalidNamedParameter = InvalidRepository.class.getMethod("findByAnnotatedQuery", String.class); + + findsProjections = ValidRepository.class.getMethod("findsProjections"); + findsProjection = ValidRepository.class.getMethod("findsProjection"); } @Test @@ -275,6 +278,21 @@ public class JpaQueryMethodUnitTests { } } + /** + * @see DATAJPA-207 + */ + @Test + @SuppressWarnings({ "rawtypes", "unchecked" }) + public void returnsTrueIfReturnTypeIsEntity() { + + when(metadata.getDomainType()).thenReturn((Class) User.class); + when(metadata.getReturnedDomainClass(findsProjections)).thenReturn((Class) Integer.class); + when(metadata.getReturnedDomainClass(findsProjection)).thenReturn((Class) Integer.class); + + assertThat(new JpaQueryMethod(findsProjections, metadata, extractor).isQueryMethodForEntity(), is(false)); + assertThat(new JpaQueryMethod(findsProjection, metadata, extractor).isQueryMethodForEntity(), is(false)); + } + /** * Interface to define invalid repository methods for testing. * @@ -322,5 +340,9 @@ public class JpaQueryMethodUnitTests { @Lock(LockModeType.PESSIMISTIC_WRITE) @Query("select u from User u where u.id = ?1") List findOneLocked(Integer primaryKey); + + List findsProjections(); + + Integer findsProjection(); } } 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 5b6821e82..6bef5f0be 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 @@ -240,4 +240,6 @@ public interface UserRepository extends JpaRepository, JpaSpecifi */ List findByFirstnameContaining(String firstname); + @Query(value = "SELECT 1 FROM User", nativeQuery = true) + List findOnesByNativeQuery(); } diff --git a/src/test/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformationIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformationIntegrationTests.java index 0740e8614..d3b11a24c 100644 --- a/src/test/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformationIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformationIntegrationTests.java @@ -89,7 +89,7 @@ public class JpaMetamodelEntityInformationIntegrationTests { SampleWithIdClass.class, em); Object id = information.getId(entity); - assertThat(id, is(SampleWithIdClassPK.class)); + assertThat(id, is(instanceOf(SampleWithIdClassPK.class))); assertThat(id, is((Object) new SampleWithIdClassPK(2L, 4L))); } } diff --git a/src/test/resources/hibernate.xml b/src/test/resources/hibernate.xml index 0936cc480..27d04d5b4 100644 --- a/src/test/resources/hibernate.xml +++ b/src/test/resources/hibernate.xml @@ -5,8 +5,7 @@ xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd http://www.springframework.org/schema/util http://www.springframework.org/schema/util/spring-util.xsd"> - +