diff --git a/src/main/java/org/springframework/data/jpa/repository/support/Querydsl.java b/src/main/java/org/springframework/data/jpa/repository/support/Querydsl.java index c75cf725d..6dc604dbd 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/Querydsl.java +++ b/src/main/java/org/springframework/data/jpa/repository/support/Querydsl.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2013 the original author or authors. + * Copyright 2012-2014 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. @@ -16,17 +16,14 @@ package org.springframework.data.jpa.repository.support; import java.util.ArrayList; -import java.util.LinkedHashSet; import java.util.List; -import java.util.Set; import javax.persistence.EntityManager; -import javax.persistence.metamodel.Attribute; -import javax.persistence.metamodel.EntityType; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Order; +import org.springframework.data.mapping.PropertyPath; import org.springframework.data.querydsl.QSort; import org.springframework.util.Assert; @@ -38,8 +35,8 @@ import com.mysema.query.jpa.impl.JPAQuery; import com.mysema.query.types.EntityPath; import com.mysema.query.types.Expression; import com.mysema.query.types.OrderSpecifier; +import com.mysema.query.types.OrderSpecifier.NullHandling; import com.mysema.query.types.Path; -import com.mysema.query.types.path.EntityPathBase; import com.mysema.query.types.path.PathBuilder; /** @@ -170,7 +167,7 @@ public class Querydsl { for (OrderSpecifier order : originalOrderSpecifiers) { - Path targetPath = ((Path) order.getTarget()).getMetadata().getParent(); + Path targetPath = ((Path) order.getTarget()).getMetadata().getParent(); boolean targetPathRootIsEntityRoot = targetPath.getRoot().equals(builder.getRoot()); boolean targetPathEqualsRootEnityPath = targetPath.toString().equals(builder.toString()); @@ -226,78 +223,35 @@ public class Querydsl { @SuppressWarnings({ "rawtypes", "unchecked" }) private OrderSpecifier toOrderSpecifier(Order order, JPQLQuery query) { - Expression property = createExpressionAndPotentionallyAddLeftJoinForReferencedAssociation(order, query); - return new OrderSpecifier(order.isAscending() ? com.mysema.query.types.Order.ASC - : com.mysema.query.types.Order.DESC, property); + : com.mysema.query.types.Order.DESC, buildOrderPropertyPathFrom(order), NullHandling.NullsLast); } /** - * Potentially adds a left join to the given {@link JPQLQuery} query if the order contains a property path that uses - * an association and returns the property expression build from the path of the association. + * Creates an {@link Expression} for the given {@link Order} property. * * @param order must not be {@literal null}. - * @param query must not be {@literal null}. - * @return property expression. - */ - private Expression createExpressionAndPotentionallyAddLeftJoinForReferencedAssociation(Order order, JPQLQuery query) { - - Assert.notNull(order, "Order must not be null!"); - Assert.notNull(query, "JPQLQuery must not be null!"); - - if (!order.getProperty().contains(".")) { - // Apply ignore case in case we have a String and ignore case ordering is requested - return order.isIgnoreCase() ? builder.getString(order.getProperty()).lower() : builder.get(order.getProperty()); - } - - EntityType entitytype = em.getMetamodel().entity(builder.getType()); - - Set> combinedAttributes = new LinkedHashSet>(); - combinedAttributes.addAll(entitytype.getSingularAttributes()); - combinedAttributes.addAll(entitytype.getPluralAttributes()); - - for (Attribute attribute : combinedAttributes) { - - if (order.getProperty().startsWith(attribute.getName() + ".")) { - - switch (attribute.getPersistentAttributeType()) { - case EMBEDDED: - return builder.get(order.getProperty()); - default: - return createLeftJoinForAttributeInOrderBy(attribute, order, query); - } - } - } - - throw new IllegalArgumentException( - String.format("Could not create property expression for %s", order.getProperty())); - } - - /** - * Adds a left-join to the given {@link JPQLQuery} with a proper alias for the property referenced on the given - * {@link Order} relative to the given parent {@link Attribute}. - * - * @param parentAttribute must not be {@literal null}. - * @param order must not be {@literal null}. - * @param query must not be {@literal null}. * @return */ - @SuppressWarnings({ "unchecked", "rawtypes" }) - private Expression createLeftJoinForAttributeInOrderBy(Attribute parentAttribute, Order order, - JPQLQuery query) { + private Expression buildOrderPropertyPathFrom(Order order) { - Assert.notNull(parentAttribute, "Attribute must not be null!"); Assert.notNull(order, "Order must not be null!"); - Assert.notNull(query, "Query must not be null!"); - EntityPathBase associationPathRoot = new EntityPathBase(parentAttribute.getJavaType(), - parentAttribute.getName()); - query.leftJoin((EntityPath) builder.get(parentAttribute.getName()), associationPathRoot); - PathBuilder attributePathBuilder = new PathBuilder(parentAttribute.getJavaType(), - associationPathRoot.getMetadata()); + PropertyPath path = PropertyPath.from(order.getProperty(), builder.getType()); + Expression sortPropertyExpression = builder; - String nestedAttributePath = order.getProperty().substring(parentAttribute.getName().length() + 1); // exclude "." - return order.isIgnoreCase() ? attributePathBuilder.getString(nestedAttributePath).lower() : attributePathBuilder - .get(nestedAttributePath); + while (path != null) { + + if (!path.hasNext() && order.isIgnoreCase()) { + // if order is ignore-case we have to treat the last path segment as a String. + sortPropertyExpression = ((PathBuilder) sortPropertyExpression).getString(path.getSegment()).lower(); + } else { + sortPropertyExpression = ((PathBuilder) sortPropertyExpression).get(path.getSegment()); + } + + path = path.next(); + } + + return sortPropertyExpression; } } diff --git a/src/test/java/org/springframework/data/jpa/domain/sample/MailSender.java b/src/test/java/org/springframework/data/jpa/domain/sample/MailSender.java index fc3051ef0..db5faf614 100644 --- a/src/test/java/org/springframework/data/jpa/domain/sample/MailSender.java +++ b/src/test/java/org/springframework/data/jpa/domain/sample/MailSender.java @@ -16,8 +16,10 @@ package org.springframework.data.jpa.domain.sample; import javax.persistence.Entity; +import javax.persistence.FetchType; import javax.persistence.GeneratedValue; import javax.persistence.Id; +import javax.persistence.ManyToOne; /** * @author Thomas Darimont @@ -29,6 +31,8 @@ public class MailSender { private String name; + @ManyToOne(fetch = FetchType.LAZY) private MailUser mailUser; + public MailSender() {} public MailSender(String name) { @@ -51,6 +55,14 @@ public class MailSender { this.id = id; } + public MailUser getMailUser() { + return mailUser; + } + + public void setMailUser(MailUser mailUser) { + this.mailUser = mailUser; + } + /* * (non-Javadoc) * @see java.lang.Object#hashCode() diff --git a/src/test/java/org/springframework/data/jpa/domain/sample/MailUser.java b/src/test/java/org/springframework/data/jpa/domain/sample/MailUser.java new file mode 100644 index 000000000..fe243980f --- /dev/null +++ b/src/test/java/org/springframework/data/jpa/domain/sample/MailUser.java @@ -0,0 +1,72 @@ +/* + * Copyright 2014 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.domain.sample; + +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; + +/** + * Represents a user in a Mail-System. + * + * @author Thomas Darimont + */ +@Entity +public class MailUser { + + @Id @GeneratedValue Long id; + + String name; + + public MailUser() {} + + public MailUser(String name) { + this.name = name; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((name == null) ? 0 : name.hashCode()); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + MailUser other = (MailUser) obj; + if (name == null) { + if (other.name != null) + return false; + } else if (!name.equals(other.name)) + return false; + return true; + } +} diff --git a/src/test/java/org/springframework/data/jpa/domain/sample/Role.java b/src/test/java/org/springframework/data/jpa/domain/sample/Role.java index e22d36a05..1a9da0c6b 100644 --- a/src/test/java/org/springframework/data/jpa/domain/sample/Role.java +++ b/src/test/java/org/springframework/data/jpa/domain/sample/Role.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2011 the original author or authors. + * Copyright 2008-2014 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. @@ -15,16 +15,22 @@ */ package org.springframework.data.jpa.domain.sample; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; + /** * Sample domain class representing roles. Mapped with XML. * * @author Oliver Gierke + * @author Thomas Darimont */ +@Entity public class Role { private static final String PREFIX = "ROLE_"; - private Integer id; + @Id @GeneratedValue private Integer id; private String name; /** diff --git a/src/test/java/org/springframework/data/jpa/domain/sample/User.java b/src/test/java/org/springframework/data/jpa/domain/sample/User.java index a2ce46402..1453c1cbb 100644 --- a/src/test/java/org/springframework/data/jpa/domain/sample/User.java +++ b/src/test/java/org/springframework/data/jpa/domain/sample/User.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2013 the original author or authors. + * Copyright 2008-2014 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. @@ -195,9 +195,9 @@ public class User { /** * Returns the user's roles. * - * @return the role + * @return the roles */ - public Set getRole() { + public Set getRoles() { return roles; } 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 e4cf534ca..a7644029b 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2013 the original author or authors. + * Copyright 2008-2014 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. @@ -85,6 +85,7 @@ public class UserRepositoryTests { // Test fixture User firstUser, secondUser, thirdUser, fourthUser; Integer id; + Role adminRole; @Before public void setUp() throws Exception { @@ -98,6 +99,7 @@ public class UserRepositoryTests { thirdUser.setAge(43); fourthUser = new User("kevin", "raymond", "no@gmail.com"); fourthUser.setAge(31); + adminRole = new Role("admin"); } @Test @@ -918,6 +920,8 @@ public class UserRepositoryTests { protected void flushTestUsers() { + em.persist(adminRole); + firstUser = repository.save(firstUser); secondUser = repository.save(secondUser); thirdUser = repository.save(thirdUser); @@ -1258,6 +1262,24 @@ public class UserRepositoryTests { assertThat(user.getEmailAddress(), is(savedUser.getEmailAddress())); } + /** + * @see DATAJPA-491 + */ + @Test + public void sortByNestedAssociationPropertyWithSortInPageable() { + + firstUser.setManager(thirdUser); + thirdUser.setManager(fourthUser); + + flushTestUsers(); + + Page page = repository.findAll(new PageRequest(0, 10, // + new Sort(Sort.Direction.ASC, "manager.manager.firstname"))); + + assertThat(page.getContent(), hasSize(4)); + assertThat(page.getContent().get(3), is(firstUser)); + } + private Page executeSpecWithSort(Sort sort) { flushTestUsers(); diff --git a/src/test/java/org/springframework/data/jpa/repository/support/MailMessageRepositoryIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/support/MailMessageRepositoryIntegrationTests.java index 73a9fbaf4..37ae4052b 100644 --- a/src/test/java/org/springframework/data/jpa/repository/support/MailMessageRepositoryIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/support/MailMessageRepositoryIntegrationTests.java @@ -21,17 +21,22 @@ import static org.springframework.data.jpa.domain.JpaSort.*; import java.util.List; +import javax.persistence.EntityManager; +import javax.persistence.PersistenceContext; + import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; +import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Direction; import org.springframework.data.jpa.domain.JpaSort; import org.springframework.data.jpa.domain.sample.MailMessage; import org.springframework.data.jpa.domain.sample.MailMessage_; import org.springframework.data.jpa.domain.sample.MailSender; import org.springframework.data.jpa.domain.sample.MailSender_; +import org.springframework.data.jpa.domain.sample.MailUser; import org.springframework.data.jpa.domain.sample.QMailMessage; import org.springframework.data.jpa.domain.sample.QMailSender; import org.springframework.data.jpa.repository.sample.MailMessageRepository; @@ -54,6 +59,8 @@ public class MailMessageRepositoryIntegrationTests { static final QMailMessage message = QMailMessage.mailMessage; static final QMailSender sender = QMailSender.mailSender; + @PersistenceContext EntityManager em; + @Autowired MailMessageRepository mailMessageRepository; /** @@ -99,7 +106,68 @@ public class MailMessageRepositoryIntegrationTests { mailMessageRepository.save(message1); mailMessageRepository.save(message2); - List messages = mailMessageRepository.findAll(message.content.eq("abc"), message.mailSender.name.asc()); + List messages = mailMessageRepository + .findAll(message.content.eq("abc"), message.mailSender.name.asc()); + + assertThat(messages, hasSize(2)); + assertThat(messages.get(0).getMailSender(), is(nullValue())); + assertThat(messages.get(1).getMailSender(), is(sender1)); + } + + /** + * @see DATAJPA-491 + */ + @Test + public void shouldSortMailWithNestedQueryDslSortCriteriaNullsFirst() { + + MailUser fooMailUser = new MailUser("foo"); + em.persist(fooMailUser); + + MailMessage message1 = new MailMessage(); + message1.setContent("abc"); + MailSender sender1 = new MailSender("foo"); + sender1.setMailUser(fooMailUser); + message1.setMailSender(sender1); + + MailMessage message2 = new MailMessage(); + message2.setContent("abc"); + + mailMessageRepository.save(message1); + mailMessageRepository.save(message2); + + List messages = mailMessageRepository.findAll(message.content.eq("abc"), + message.mailSender.mailUser.name.asc()); + + assertThat(messages, hasSize(2)); + assertThat(messages.get(0).getMailSender(), is(nullValue())); + assertThat(messages.get(1).getMailSender(), is(sender1)); + } + + /** + * @see DATAJPA-491 + */ + @Test + public void shouldSortMailWithNestedStringBasedSortCriteriaNullsFirst() { + + MailUser fooMailUser = new MailUser("foo"); + em.persist(fooMailUser); + + MailMessage message1 = new MailMessage(); + message1.setContent("abc"); + MailSender sender1 = new MailSender("foo"); + sender1.setMailUser(fooMailUser); + message1.setMailSender(sender1); + + MailMessage message2 = new MailMessage(); + message2.setContent("abc"); + + mailMessageRepository.save(message1); + mailMessageRepository.save(message2); + + Page page = mailMessageRepository.findAll(new PageRequest(0, 10, new Sort(Sort.Direction.ASC, + "mailSender.mailUser.name"))); + + List messages = page.getContent(); assertThat(messages, hasSize(2)); assertThat(messages.get(0).getMailSender(), is(nullValue())); diff --git a/src/test/java/org/springframework/data/jpa/repository/support/QueryDslJpaRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/support/QueryDslJpaRepositoryTests.java index 0ef5d2914..d4b2f14bf 100644 --- a/src/test/java/org/springframework/data/jpa/repository/support/QueryDslJpaRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/support/QueryDslJpaRepositoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2011 the original author or authors. + * Copyright 2008-2014 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. @@ -33,6 +33,7 @@ import org.springframework.data.domain.Sort.Direction; import org.springframework.data.domain.Sort.Order; import org.springframework.data.jpa.domain.sample.Address; import org.springframework.data.jpa.domain.sample.QUser; +import org.springframework.data.jpa.domain.sample.Role; import org.springframework.data.jpa.domain.sample.User; import org.springframework.data.querydsl.QPageRequest; import org.springframework.data.querydsl.QSort; @@ -49,6 +50,7 @@ import com.mysema.query.types.path.PathBuilderFactory; * Integration test for {@link QueryDslJpaRepository}. * * @author Oliver Gierke + * @author Thomas Darimont */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration({ "classpath:infrastructure.xml" }) @@ -60,6 +62,7 @@ public class QueryDslJpaRepositoryTests { QueryDslJpaRepository repository; QUser user = new QUser("user"); User dave, carter, oliver; + Role adminRole; @Before public void setUp() { @@ -71,6 +74,7 @@ public class QueryDslJpaRepositoryTests { dave = repository.save(new User("Dave", "Matthews", "dave@matthews.com")); carter = repository.save(new User("Carter", "Beauford", "carter@beauford.com")); oliver = repository.save(new User("Oliver", "matthews", "oliver@matthews.com")); + adminRole = em.merge(new Role("admin")); } @Test @@ -273,4 +277,20 @@ public class QueryDslJpaRepositoryTests { assertThat(page.getContent().get(1), is(dave)); assertThat(page.getContent().get(2), is(oliver)); } + + /** + * @see DATAJPA-491 + */ + @Test + public void sortByNestedAssociationPropertyWithSpecificationAndSortInPageable() { + + oliver.setManager(dave); + dave.getRoles().add(adminRole); + + Page page = repository.findAll(QUser.user.id.gt(0), new PageRequest(0, 10, // + new Sort(Sort.Direction.ASC, "manager.roles.name"))); + + assertThat(page.getContent(), hasSize(3)); + assertThat(page.getContent().get(0), is(dave)); + } } diff --git a/src/test/resources/META-INF/persistence.xml b/src/test/resources/META-INF/persistence.xml index a67914334..d2b44c3ea 100644 --- a/src/test/resources/META-INF/persistence.xml +++ b/src/test/resources/META-INF/persistence.xml @@ -30,12 +30,14 @@ org.springframework.data.jpa.domain.sample.Address org.springframework.data.jpa.domain.sample.MailMessage org.springframework.data.jpa.domain.sample.MailSender + org.springframework.data.jpa.domain.sample.MailUser true org.springframework.data.jpa.domain.sample.User org.springframework.data.jpa.domain.sample.MailMessage org.springframework.data.jpa.domain.sample.MailSender + org.springframework.data.jpa.domain.sample.MailUser true @@ -44,6 +46,7 @@ org.springframework.data.jpa.repository.cdi.Person org.springframework.data.jpa.domain.sample.MailMessage org.springframework.data.jpa.domain.sample.MailSender + org.springframework.data.jpa.domain.sample.MailUser true @@ -74,6 +77,7 @@ org.springframework.data.jpa.domain.sample.User 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.repository.support.JpaMetamodelEntityInformationIntegrationTests$Sample true @@ -85,6 +89,7 @@ org.springframework.data.jpa.domain.sample.User 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.repository.support.JpaMetamodelEntityInformationIntegrationTests$Sample true @@ -93,6 +98,7 @@ org.springframework.data.jpa.domain.sample.User 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.repository.support.JpaMetamodelEntityInformationIntegrationTests$Sample true diff --git a/src/test/resources/META-INF/persistence2.xml b/src/test/resources/META-INF/persistence2.xml index d7caad1cc..1c7fd7a17 100644 --- a/src/test/resources/META-INF/persistence2.xml +++ b/src/test/resources/META-INF/persistence2.xml @@ -8,6 +8,7 @@ org.springframework.data.jpa.domain.sample.Role org.springframework.data.jpa.domain.sample.MailMessage org.springframework.data.jpa.domain.sample.MailSender + org.springframework.data.jpa.domain.sample.MailUser true @@ -18,6 +19,7 @@ org.springframework.data.jpa.domain.sample.AuditableRole org.springframework.data.jpa.domain.sample.MailMessage org.springframework.data.jpa.domain.sample.MailSender + org.springframework.data.jpa.domain.sample.MailUser true