From 79b5330928e0e3f77d974acbe4dfffbb3e258bb3 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Mon, 10 Mar 2014 12:13:50 +0100 Subject: [PATCH] DATAJPA-491 - Support order by arbitrarily nested association paths with Querydsl. Replaced custom left join generation logic with default Querydsl mechanisms including support for ordering by arbitrarily nested property paths. Added test cases that demonstrate ordering by nested association paths (>= 2 levels). Added additional test cases for sort by nested property path expressions based on querydsl meta model and plain string based path expressions. Original pull request: #65. --- .../data/jpa/repository/support/Querydsl.java | 90 +++++-------------- .../data/jpa/domain/sample/MailSender.java | 12 +++ .../data/jpa/domain/sample/MailUser.java | 72 +++++++++++++++ .../data/jpa/domain/sample/Role.java | 10 ++- .../data/jpa/domain/sample/User.java | 6 +- .../jpa/repository/UserRepositoryTests.java | 24 ++++- ...MailMessageRepositoryIntegrationTests.java | 70 ++++++++++++++- .../support/QueryDslJpaRepositoryTests.java | 22 ++++- src/test/resources/META-INF/persistence.xml | 6 ++ src/test/resources/META-INF/persistence2.xml | 2 + 10 files changed, 238 insertions(+), 76 deletions(-) create mode 100644 src/test/java/org/springframework/data/jpa/domain/sample/MailUser.java 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