From e224d8ccceb547961f94a3a108bf3b03bd4b35b3 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 explicite 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 | 100 +++++++++++++++--- .../domain/sample/IdClassExampleEmployee.java | 3 +- ...lipseLinkNamespaceUserRepositoryTests.java | 10 ++ .../jpa/repository/UserRepositoryTests.java | 23 +++- .../cdi/EntityManagerFactoryProducer.java | 12 ++- .../query/TupleConverterUnitTests.java | 97 +++++++++++++++-- .../jpa/repository/sample/UserRepository.java | 8 ++ ...odelEntityInformationIntegrationTests.java | 30 +----- src/test/resources/META-INF/persistence.xml | 32 ------ 9 files changed, 227 insertions(+), 88 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 56c646f46..4ca6a80dc 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.List; import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; import javax.persistence.EntityManager; import javax.persistence.LockModeType; @@ -285,19 +289,7 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { } } - Map result = new HashMap<>(); - 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) { @@ -309,5 +301,87 @@ 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.stream(tuple.toArray()).anyMatch(v -> v.equals(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 TupleBakcedMap cannot be modified"); + } + + @Override + public Object remove(Object key) { + throw new UnsupportedOperationException("A TupleBakcedMap cannot be modified"); + } + + @Override + public void putAll(Map m) { + throw new UnsupportedOperationException("A TupleBakcedMap cannot be modified"); + } + + @Override + public void clear() { + throw new UnsupportedOperationException("A TupleBakcedMap cannot be modified"); + } + + @Override + public Set keySet() { + + return tuple.getElements().stream() // + .map(TupleElement::getAlias) // + .collect(Collectors.toSet()); + } + + @Override + public Collection values() { + return Arrays.asList(tuple.toArray()); + } + + @Override + public Set> entrySet() { + return tuple.getElements().stream() // + .map(e -> new HashMap.SimpleEntry(e.getAlias(), tuple.get(e))) // + .collect(Collectors.toSet()); + } + } } } 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 7c2375745..2323d8453 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 bfc3780c5..013ba7be6 100644 --- a/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java @@ -94,4 +94,14 @@ public class EclipseLinkNamespaceUserRepositoryTests extends NamespaceUserReposi @Override @Test // DATAJPA-980 public void supportsProjectionsWithNativeQueries() {} + + /** + * 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 a2543a023..1170d383b 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -50,8 +50,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; @@ -60,6 +58,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; @@ -2059,8 +2058,24 @@ public class UserRepositoryTests { assertThat(result.getLastname()).isEqualTo(user.getLastname()); } - @Test //DATAJPA-1235 - public void handlesColonsFollowedByIntegerInStringLiteral(){ + @Test // DATAJPA-1248 + public void supportsProjectionsWithNativeQueriesAndCamelCaseProperty() { + + flushTestUsers(); + User user = repository.findAll().get(0); + + UserRepository.EmailOnly result = repository.findEmailOnlyByNativeQuery(user.getId()); + + String emailAddress = result.getEmailAddress(); + + assertThat(emailAddress) // + .isEqualTo(user.getEmailAddress()) // + .as("ensuring email is actually not null") // + .isNotNull(); + } + + @Test // DATAJPA-1235 + public void handlesColonsFollowedByIntegerInStringLiteral() { String firstName = "aFirstName"; 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 0db82b39c..c6574b7ab 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 8f09687ed..5b52a1b27 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 66944af65..ede49727c 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 @@ -509,6 +509,10 @@ public interface UserRepository @Query(value = "SELECT firstname, lastname FROM SD_User WHERE id = ?1", nativeQuery = true) NameOnly findByNativeQuery(Integer id); + // DATAJPA-1248 + @Query(value = "SELECT emailaddress FROM SD_User WHERE id = ?1", nativeQuery = true) + EmailOnly findEmailOnlyByNativeQuery(Integer id); + // DATAJPA-1235 @Query("SELECT u FROM User u where u.firstname >= ?1 and u.lastname = '000:1'") List queryWithIndexedParameterAndColonFollowedByIntegerInString(String firstname); @@ -526,4 +530,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 050c34162..d1f9d0093 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 @@ -23,38 +23,14 @@ import java.io.Serializable; import java.sql.Timestamp; import java.util.Date; -import javax.persistence.Access; -import javax.persistence.AccessType; -import javax.persistence.Entity; -import javax.persistence.EntityManager; -import javax.persistence.EntityManagerFactory; -import javax.persistence.Id; -import javax.persistence.IdClass; -import javax.persistence.MappedSuperclass; -import javax.persistence.Persistence; -import javax.persistence.PersistenceContext; +import javax.persistence.*; 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; -import org.springframework.data.jpa.domain.sample.ItemId; -import org.springframework.data.jpa.domain.sample.ItemSite; -import org.springframework.data.jpa.domain.sample.ItemSiteId; -import org.springframework.data.jpa.domain.sample.PersistableWithIdClass; -import org.springframework.data.jpa.domain.sample.PersistableWithIdClassPK; -import org.springframework.data.jpa.domain.sample.PrimitiveVersionProperty; -import org.springframework.data.jpa.domain.sample.Role; -import org.springframework.data.jpa.domain.sample.SampleWithIdClass; -import org.springframework.data.jpa.domain.sample.SampleWithPrimitiveId; -import org.springframework.data.jpa.domain.sample.SampleWithTimestampVersion; -import org.springframework.data.jpa.domain.sample.Site; -import org.springframework.data.jpa.domain.sample.User; -import org.springframework.data.jpa.domain.sample.VersionedUser; +import org.springframework.data.jpa.domain.sample.*; import org.springframework.data.repository.core.EntityInformation; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -283,7 +259,7 @@ public class JpaMetamodelEntityInformationIntegrationTests { } protected String getMetadadataPersitenceUnitName() { - return Version.getVersionString().startsWith("5.2") ? "metadata-52" : "metadata"; + return "metadata"; } @SuppressWarnings("serial") diff --git a/src/test/resources/META-INF/persistence.xml b/src/test/resources/META-INF/persistence.xml index 5c24cbe85..eae1b2ecc 100644 --- a/src/test/resources/META-INF/persistence.xml +++ b/src/test/resources/META-INF/persistence.xml @@ -54,25 +54,6 @@ true - org.hibernate.ejb.HibernatePersistence - org.springframework.data.jpa.domain.sample.MailMessage - org.springframework.data.jpa.domain.sample.MailSender - org.springframework.data.jpa.domain.sample.MailUser - org.springframework.data.jpa.domain.sample.User - org.springframework.data.jpa.repository.cdi.Person - org.springframework.data.jpa.domain.sample.Dummy - true - - - - - - - - - - - org.hibernate.jpa.HibernatePersistenceProvider org.springframework.data.jpa.domain.sample.MailMessage org.springframework.data.jpa.domain.sample.MailSender @@ -106,19 +87,6 @@ - org.hibernate.ejb.HibernatePersistence - org.springframework.data.jpa.domain.sample.CustomAbstractPersistable - org.springframework.data.jpa.domain.sample.MailMessage - org.springframework.data.jpa.domain.sample.MailSender - org.springframework.data.jpa.domain.sample.MailUser - org.springframework.data.jpa.domain.sample.User - org.springframework.data.jpa.repository.support.JpaMetamodelEntityInformationIntegrationTests$Sample - true - - - - - org.hibernate.jpa.HibernatePersistenceProvider org.springframework.data.jpa.domain.sample.CustomAbstractPersistable org.springframework.data.jpa.domain.sample.MailMessage