From 4d62b90f797792739bc3aa6ca950da4e2e7d07db Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Wed, 24 Jan 2018 06:13:36 +0100 Subject: [PATCH] DATAJPA-1248 - Fixed build for Hibernate 5.3. Removed obsolete version detection and associated persistence units from tests. Added explicit cascade option to many-to-one reference in id. It is not clear if this is a bug in Hibernate or not. See HHH-12251 for details. The fix of HHH-12119 made it obvious that our use of Tuples for Projections from native queries were not correct. When looking up Tuple values we now find them using a property name with correct upper/lower case as well as with the upper case as it is actually returned by the JDBC driver. Converted all touched tests using Hamcrest to use AssertJ instead. See also: https://hibernate.atlassian.net/browse/HHH-12251 https://hibernate.atlassian.net/browse/HHH-12119 Original pull request: #245. --- .../repository/query/AbstractJpaQuery.java | 110 +++++++++++++++--- .../domain/sample/IdClassExampleEmployee.java | 3 +- ...lipseLinkNamespaceUserRepositoryTests.java | 10 ++ .../jpa/repository/UserRepositoryTests.java | 38 +++++- .../cdi/EntityManagerFactoryProducer.java | 12 +- .../query/TupleConverterUnitTests.java | 97 ++++++++++++++- .../jpa/repository/sample/UserRepository.java | 8 ++ ...odelEntityInformationIntegrationTests.java | 4 +- 8 files changed, 250 insertions(+), 32 deletions(-) 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 43eadff72..4a7a1527c 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 @@ -15,9 +15,13 @@ */ package org.springframework.data.jpa.repository.query; +import java.util.Arrays; +import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import javax.persistence.EntityManager; import javax.persistence.LockModeType; @@ -263,7 +267,6 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { } Tuple tuple = (Tuple) source; - Map result = new HashMap(); List> elements = tuple.getElements(); if (elements.size() == 1) { @@ -275,18 +278,7 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { } } - for (TupleElement element : elements) { - - String alias = element.getAlias(); - - if (alias == null || isIndexAsString(alias)) { - throw new IllegalStateException("No aliases found in result tuple! Make sure your query defines aliases!"); - } - - result.put(element.getAlias(), tuple.get(element)); - } - - return result; + return new TupleBackedMap(tuple); } private static boolean isIndexAsString(String source) { @@ -298,5 +290,97 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { return false; } } + + /** + * A {@link Map} implementation which delegates all calls to a {@link Tuple}. Depending on the provided + * {@link Tuple} implementation it might return the same value for various keys of which only one will appear in the + * key/entry set. + * + * @author Jens Schauder + */ + private static class TupleBackedMap implements Map { + + private final Tuple tuple; + + TupleBackedMap(Tuple tuple) { + this.tuple = tuple; + } + + @Override + public int size() { + return tuple.getElements().size(); + } + + @Override + public boolean isEmpty() { + return tuple.getElements().isEmpty(); + } + + @Override + public boolean containsKey(Object key) { + return key instanceof String && tuple.get((String) key) != null; + } + + @Override + public boolean containsValue(Object value) { + return Arrays.asList(tuple.toArray()).contains(value); + } + + @Override + public Object get(Object key) { + return key instanceof String ? tuple.get((String) key) : null; + } + + @Override + public Object put(String key, Object value) { + throw new UnsupportedOperationException("A TupleBackedMap cannot be modified"); + } + + @Override + public Object remove(Object key) { + throw new UnsupportedOperationException("A TupleBackedMap cannot be modified"); + } + + @Override + public void putAll(Map m) { + throw new UnsupportedOperationException("A TupleBackedMap cannot be modified"); + } + + @Override + public void clear() { + throw new UnsupportedOperationException("A TupleBackedMap cannot be modified"); + } + + @Override + public Set keySet() { + + List> elements = tuple.getElements(); + Set result = new HashSet(elements.size()); + + for (TupleElement element : elements) { + result.add(element.getAlias()); + } + + return result; + } + + @Override + public Collection values() { + return Arrays.asList(tuple.toArray()); + } + + @Override + public Set> entrySet() { + + List> elements = tuple.getElements(); + Set> result = new HashSet>(elements.size()); + + for (TupleElement element : elements) { + result.add(new HashMap.SimpleEntry(element.getAlias(), tuple.get(element))); + } + + return result; + } + } } } diff --git a/src/test/java/org/springframework/data/jpa/domain/sample/IdClassExampleEmployee.java b/src/test/java/org/springframework/data/jpa/domain/sample/IdClassExampleEmployee.java index 8ab1136d6..245af0316 100644 --- a/src/test/java/org/springframework/data/jpa/domain/sample/IdClassExampleEmployee.java +++ b/src/test/java/org/springframework/data/jpa/domain/sample/IdClassExampleEmployee.java @@ -15,6 +15,7 @@ */ package org.springframework.data.jpa.domain.sample; +import javax.persistence.CascadeType; import javax.persistence.Entity; import javax.persistence.Id; import javax.persistence.IdClass; @@ -29,7 +30,7 @@ import javax.persistence.ManyToOne; public class IdClassExampleEmployee { @Id long empId; - @Id @ManyToOne IdClassExampleDepartment department; + @Id @ManyToOne(cascade = CascadeType.ALL) IdClassExampleDepartment department; String name; diff --git a/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java index dd0a81245..f3953365c 100644 --- a/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java @@ -92,4 +92,14 @@ public class EclipseLinkNamespaceUserRepositoryTests extends NamespaceUserReposi Query query = em.createNativeQuery("select 1 from User where firstname=? and lastname=?"); assertThat(query.getParameters().size(), equalTo(0)); } + + /** + * Ignored until https://bugs.eclipse.org/bugs/show_bug.cgi?id=525319 is fixed. + */ + @Ignore + @Override + @Test // DATAJPA-1248 + public void supportsProjectionsWithNativeQueriesAndCamelCaseProperty() { + super.supportsProjectionsWithNativeQueriesAndCamelCaseProperty(); + } } 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 c36cdf734..bc3d616da 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -45,7 +45,6 @@ import javax.persistence.criteria.Predicate; import javax.persistence.criteria.Root; import org.hamcrest.Matchers; -import org.hibernate.Version; import org.junit.Assume; import org.junit.Before; import org.junit.Ignore; @@ -57,8 +56,6 @@ import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.data.domain.Example; import org.springframework.data.domain.ExampleMatcher; -import org.springframework.data.domain.ExampleMatcher.GenericPropertyMatcher; -import org.springframework.data.domain.ExampleMatcher.StringMatcher; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.PageRequest; @@ -67,6 +64,7 @@ import org.springframework.data.domain.Slice; import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Direction; import org.springframework.data.domain.Sort.Order; +import org.springframework.data.domain.ExampleMatcher.*; import org.springframework.data.jpa.domain.Specification; import org.springframework.data.jpa.domain.sample.Address; import org.springframework.data.jpa.domain.sample.Role; @@ -76,6 +74,8 @@ import org.springframework.data.jpa.provider.PersistenceProvider; import org.springframework.data.jpa.repository.sample.SampleEvaluationContextExtension.SampleSecurityContextHolder; import org.springframework.data.jpa.repository.sample.UserRepository; import org.springframework.data.jpa.repository.sample.UserRepository.NameOnly; +import org.springframework.data.projection.TargetAware; +import org.springframework.data.util.Version; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.transaction.annotation.Transactional; @@ -99,6 +99,8 @@ import com.google.common.base.Optional; @Transactional public class UserRepositoryTests { + private static final Version HIBERNATE_VERSION_SUPPORTING_TUPLE_ON_NATIVE_QUERIES = new Version(5, 2, 11); + @PersistenceContext EntityManager em; // CUT @@ -2182,7 +2184,8 @@ public class UserRepositoryTests { @Test // DATAJPA-980 public void supportsProjectionsWithNativeQueries() { - Assume.assumeTrue(Version.getVersionString().startsWith("5.2")); + Assume + .assumeTrue(getHibernateVersion().isGreaterThanOrEqualTo(HIBERNATE_VERSION_SUPPORTING_TUPLE_ON_NATIVE_QUERIES)); flushTestUsers(); @@ -2194,6 +2197,27 @@ public class UserRepositoryTests { assertThat(result.getLastname(), is(user.getLastname())); } + @Test // DATAJPA-1248 + public void supportsProjectionsWithNativeQueriesAndCamelCaseProperty() throws Exception { + + Assume + .assumeTrue(getHibernateVersion().isGreaterThanOrEqualTo(HIBERNATE_VERSION_SUPPORTING_TUPLE_ON_NATIVE_QUERIES)); + + flushTestUsers(); + User user = repository.findAll().get(0); + + UserRepository.EmailOnly result = repository.findEmailOnlyByNativeQuery(user.getId()); + + System.out.println(((TargetAware) result).getTarget()); + + String emailAddress = result.getEmailAddress(); + + assertThat(emailAddress) // + .isEqualTo(user.getEmailAddress()) // + .as("ensuring email is actually not null") // + .isNotNull(); + } + private Page executeSpecWithSort(Sort sort) { flushTestUsers(); @@ -2204,4 +2228,10 @@ public class UserRepositoryTests { assertThat(result.getTotalElements(), is(2L)); return result; } + + private static Version getHibernateVersion() { + + String hibernateVersion = org.hibernate.Version.getVersionString(); + return Version.parse(hibernateVersion.substring(0, hibernateVersion.lastIndexOf("."))); + } } diff --git a/src/test/java/org/springframework/data/jpa/repository/cdi/EntityManagerFactoryProducer.java b/src/test/java/org/springframework/data/jpa/repository/cdi/EntityManagerFactoryProducer.java index c06fcd451..d91969875 100644 --- a/src/test/java/org/springframework/data/jpa/repository/cdi/EntityManagerFactoryProducer.java +++ b/src/test/java/org/springframework/data/jpa/repository/cdi/EntityManagerFactoryProducer.java @@ -21,16 +21,18 @@ import javax.enterprise.inject.Produces; import javax.persistence.EntityManagerFactory; import javax.persistence.Persistence; -import org.hibernate.Version; - +/** + * Produces and {@link EntityManagerFactory}. + * + * @author Dirk Mahler + * @author Jens Schauder + */ class EntityManagerFactoryProducer { @Produces @ApplicationScoped public EntityManagerFactory createEntityManagerFactory() { - - String hibernateVersion = Version.getVersionString(); - return Persistence.createEntityManagerFactory(hibernateVersion.startsWith("5.2") ? "cdi-52" : "cdi"); + return Persistence.createEntityManagerFactory("cdi"); } public void close(@Disposes EntityManagerFactory entityManagerFactory) { diff --git a/src/test/java/org/springframework/data/jpa/repository/query/TupleConverterUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/TupleConverterUnitTests.java index 4e6b17802..4bffeee3b 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/TupleConverterUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/TupleConverterUnitTests.java @@ -15,15 +15,18 @@ */ package org.springframework.data.jpa.repository.query; -import static org.hamcrest.CoreMatchers.*; -import static org.junit.Assert.*; +import static org.assertj.core.api.Assertions.*; import static org.mockito.Mockito.*; import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; import javax.persistence.Tuple; import javax.persistence.TupleElement; +import org.assertj.core.api.SoftAssertions; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -41,6 +44,7 @@ import org.springframework.data.repository.query.ReturnedType; * Unit tests for {@link TupleConverter}. * * @author Oliver Gierke + * @author Jens Schauder * @soundtrack James Bay - Let it go (Chaos and the Calm) */ @RunWith(MockitoJUnitRunner.class) @@ -70,22 +74,103 @@ public class TupleConverterUnitTests { TupleConverter converter = new TupleConverter(type); - assertThat(converter.convert(tuple), is((Object) "Foo")); + assertThat(converter.convert(tuple)).isEqualTo("Foo"); } @Test // DATAJPA-1024 @SuppressWarnings("unchecked") public void returnsNullForSingleElementTupleWithNullValue() throws Exception { - doReturn(Arrays.asList(element)).when(tuple).getElements(); + doReturn(Collections.singletonList(element)).when(tuple).getElements(); doReturn(null).when(tuple).get(element); TupleConverter converter = new TupleConverter(type); - assertThat(converter.convert(tuple), is(nullValue())); + assertThat(converter.convert(tuple)).isNull(); } - static interface SampleRepository extends CrudRepository { + @SuppressWarnings("unchecked") + @Test // DATAJPA-1048 + public void findsValuesForAllVariantsSupportedByTheTuple() { + + Tuple tuple = new MockTuple(); + + TupleConverter converter = new TupleConverter(type); + + Map map = (Map) converter.convert(tuple); + + SoftAssertions softly = new SoftAssertions(); + + softly.assertThat(map.get("ONE")).isEqualTo("one"); + softly.assertThat(map.get("one")).isEqualTo("one"); + softly.assertThat(map.get("OnE")).isEqualTo("one"); + softly.assertThat(map.get("oNe")).isEqualTo("one"); + + softly.assertAll(); + } + + interface SampleRepository extends CrudRepository { String someMethod(); } + + @SuppressWarnings("unchecked") + private static class MockTuple implements Tuple { + + TupleElement one = new StringTupleElement("oNe"); + TupleElement two = new StringTupleElement("tWo"); + + @Override + public X get(TupleElement tupleElement) { + return (X) get(tupleElement.getAlias()); + } + + @Override + public X get(String alias, Class type) { + return (X) get(alias); + } + + @Override + public Object get(String alias) { + return alias.toLowerCase(); + } + + @Override + public X get(int i, Class type) { + return (X) String.valueOf(i); + } + + @Override + public Object get(int i) { + return get(i, Object.class); + } + + @Override + public Object[] toArray() { + return new Object[] { one.getAlias().toLowerCase(), two.getAlias().toLowerCase() }; + } + + @Override + public List> getElements() { + return Arrays.asList(one, two); + } + + private static class StringTupleElement implements TupleElement { + + private final String value; + + private StringTupleElement(String value) { + this.value = value; + } + + @Override + public Class getJavaType() { + return String.class; + } + + @Override + public String getAlias() { + return value; + } + } + } } 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 7262f929d..dfb7cd2ed 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 @@ -481,6 +481,10 @@ public interface UserRepository // DATAJPA-1185 List findAsListByFirstnameLike(String name, Class projectionType); + // DATAJPA-1248 + @Query(value = "SELECT emailaddress FROM SD_User WHERE id = ?1", nativeQuery = true) + EmailOnly findEmailOnlyByNativeQuery(Integer id); + interface RolesAndFirstname { @@ -495,4 +499,8 @@ public interface UserRepository String getLastname(); } + + interface EmailOnly { + String getEmailAddress(); + } } 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 4d4fd915a..6a31d9475 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 @@ -35,11 +35,9 @@ import javax.persistence.Persistence; import javax.persistence.PersistenceContext; import javax.persistence.metamodel.Metamodel; -import org.hibernate.Version; import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; - import org.springframework.data.jpa.domain.AbstractPersistable; import org.springframework.data.jpa.domain.sample.ConcreteType1; import org.springframework.data.jpa.domain.sample.Item; @@ -288,7 +286,7 @@ public class JpaMetamodelEntityInformationIntegrationTests { } protected String getMetadadataPersitenceUnitName() { - return Version.getVersionString().startsWith("5.2") ? "metadata-52" : "metadata"; + return "metadata"; } @SuppressWarnings("serial")