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 1186ba9fd..6efedc31d 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 @@ -15,7 +15,12 @@ */ package org.springframework.data.jpa.repository.support; +import java.util.LinkedHashSet; +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; @@ -30,6 +35,7 @@ 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.path.EntityPathBase; import com.mysema.query.types.path.PathBuilder; /** @@ -121,7 +127,7 @@ public class Querydsl { } for (Order order : sort) { - query.orderBy(toOrder(order)); + query.orderBy(toOrder(order, query)); } return query; @@ -134,16 +140,71 @@ public class Querydsl { * @return */ @SuppressWarnings({ "rawtypes", "unchecked" }) - private OrderSpecifier toOrder(Order order) { + private OrderSpecifier toOrder(Order order, JPQLQuery query) { - Expression property = builder.get(order.getProperty()); - - // Apply ignore case in case we have a String and ignore case ordering is requested - if (order.isIgnoreCase()) { - property = builder.getString(order.getProperty()).lower(); - } + Expression property = createExpressionAndPotentionallyAddLeftJoinForReferencedAssociation(order, query); return new OrderSpecifier(order.isAscending() ? com.mysema.query.types.Order.ASC : com.mysema.query.types.Order.DESC, property); } + + /** + * 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. + * + * @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())); + } + + /** + * @param attribute + * @param order + * @param query + * @return + */ + @SuppressWarnings({ "unchecked", "rawtypes" }) + private Expression createLeftJoinForAttributeInOrderBy(Attribute attribute, Order order, JPQLQuery query) { + + EntityPathBase associationPathRoot = new EntityPathBase(attribute.getJavaType(), attribute.getName()); + query.leftJoin((EntityPath) builder.get(attribute.getName()), associationPathRoot); + PathBuilder attributePathBuilder = new PathBuilder(attribute.getJavaType(), + associationPathRoot.getMetadata()); + + String nestedAttributePath = order.getProperty().substring(attribute.getName().length() + 1); // exclude "." + return order.isIgnoreCase() ? attributePathBuilder.getString(nestedAttributePath).lower() : attributePathBuilder + .get(nestedAttributePath); + } } diff --git a/src/test/java/org/springframework/data/jpa/domain/sample/Address.java b/src/test/java/org/springframework/data/jpa/domain/sample/Address.java new file mode 100644 index 000000000..e7b93eca1 --- /dev/null +++ b/src/test/java/org/springframework/data/jpa/domain/sample/Address.java @@ -0,0 +1,55 @@ +/* + * Copyright 2013 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.Embeddable; + +/** + * @author Thomas Darimont + */ +@Embeddable +public class Address { + + private String country; + private String city; + private String streetName; + private String streetNo; + + public Address() {} + + public Address(String country, String city, String streetName, String streetNo) { + this.country = country; + this.city = city; + this.streetName = streetName; + this.streetNo = streetNo; + } + + public String getCountry() { + return country; + } + + public String getCity() { + return city; + } + + public String getStreetName() { + return streetName; + } + + public String getStreetNo() { + return streetNo; + } +} 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 a9022d1ac..0a99c7678 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 @@ -21,6 +21,7 @@ import java.util.Set; import javax.persistence.CascadeType; import javax.persistence.Column; +import javax.persistence.Embedded; import javax.persistence.Entity; import javax.persistence.GeneratedValue; import javax.persistence.GenerationType; @@ -63,6 +64,8 @@ public class User { @ManyToOne private User manager; + @Embedded private Address address; + /** * Creates a new empty instance of {@code User}. */ @@ -281,6 +284,20 @@ public class User { return createdAt; } + /** + * @return the address + */ + public Address getAddress() { + return address; + } + + /** + * @param address the address to set + */ + public void setAddress(Address address) { + this.address = address; + } + /* * (non-Javadoc) * 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 7327b1dcf..5e9d99450 100644 --- a/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java @@ -63,4 +63,16 @@ public class EclipseLinkNamespaceUserRepositoryTests extends NamespaceUserReposi public void shouldGenerateLeftOuterJoinInfindAllWithPaginationAndSortOnNestedPropertyPath() { super.shouldGenerateLeftOuterJoinInfindAllWithPaginationAndSortOnNestedPropertyPath(); } + + /** + * Ignored until https://bugs.eclipse.org/bugs/show_bug.cgi?id=422450 is resolved. + */ + @Override + public void sortByAssociationPropertyShouldUseLeftOuterJoin() {} + + /** + * Ignored until https://bugs.eclipse.org/bugs/show_bug.cgi?id=422450 is resolved. + */ + @Override + public void sortByAssociationPropertyInPageableShouldUseLeftOuterJoin() {} } 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 dde7a5828..4fa5aa49a 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -53,6 +53,7 @@ import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Direction; import org.springframework.data.domain.Sort.Order; import org.springframework.data.jpa.domain.Specification; +import org.springframework.data.jpa.domain.sample.Address; import org.springframework.data.jpa.domain.sample.Role; import org.springframework.data.jpa.domain.sample.User; import org.springframework.data.jpa.repository.sample.UserRepository; @@ -68,6 +69,7 @@ import org.springframework.transaction.annotation.Transactional; * * @author Oliver Gierke * @author Kevin Raymond + * @author Thomas Darimont */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration("classpath:application-context.xml") @@ -1107,6 +1109,51 @@ public class UserRepositoryTests { assertThat(result, contains(secondUser, firstUser, thirdUser, fourthUser)); } + /** + * @see DATAJPA-427 + */ + @Test + public void sortByAssociationPropertyShouldUseLeftOuterJoin() { + + secondUser.getColleagues().add(firstUser); + fourthUser.getColleagues().add(thirdUser); + flushTestUsers(); + + List result = repository.findAll(new Sort(Sort.Direction.ASC, "colleagues.id")); + + assertThat(result, hasSize(4)); + } + + /** + * @see DATAJPA-427 + */ + @Test + public void sortByAssociationPropertyInPageableShouldUseLeftOuterJoin() { + + secondUser.getColleagues().add(firstUser); + fourthUser.getColleagues().add(thirdUser); + flushTestUsers(); + + Page page = repository.findAll(new PageRequest(0, 10, new Sort(Sort.Direction.ASC, "colleagues.id"))); + + assertThat(page.getContent(), hasSize(4)); + } + + /** + * @see DATAJPA-427 + */ + @Test + public void sortByEmbeddedProperty() { + + thirdUser.setAddress(new Address("Germany", "Saarbrücken", "HaveItYourWay", "123")); + flushTestUsers(); + + Page page = repository.findAll(new PageRequest(0, 10, new Sort(Sort.Direction.ASC, "address.streetName"))); + + assertThat(page.getContent(), hasSize(4)); + assertThat(page.getContent().get(3), is(thirdUser)); + } + private Page executeSpecWithSort(Sort sort) { flushTestUsers(); 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 ae92839e8..06c24d4ad 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 @@ -31,6 +31,7 @@ import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Sort; 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.User; import org.springframework.test.context.ContextConfiguration; @@ -52,8 +53,7 @@ import com.mysema.query.types.path.PathBuilderFactory; @Transactional public class QueryDslJpaRepositoryTests { - @PersistenceContext - EntityManager em; + @PersistenceContext EntityManager em; QueryDslJpaRepository repository; QUser user = new QUser("user"); @@ -131,4 +131,88 @@ public class QueryDslJpaRepositoryTests { assertThat(result.getContent().get(0), is(dave)); assertThat(result.getContent().get(1), is(oliver)); } + + /** + * @see DATAJPA-427 + */ + @Test + public void findBySpecificationWithSortByPluralAssociationPropertyInPageableShouldUseSortNullValuesLast() { + + oliver.getColleagues().add(dave); + dave.getColleagues().add(oliver); + + QUser user = QUser.user; + + Page page = repository.findAll(user.firstname.isNotNull(), new PageRequest(0, 10, new Sort( + Sort.Direction.ASC, "colleagues.firstname"))); + + assertThat(page.getContent(), hasSize(3)); + assertThat(page.getContent(), hasItems(oliver, dave, carter)); + } + + /** + * @see DATAJPA-427 + */ + @Test + public void findBySpecificationWithSortBySingularAssociationPropertyInPageableShouldUseSortNullValuesLast() { + + oliver.setManager(dave); + dave.setManager(carter); + + QUser user = QUser.user; + + Page page = repository.findAll(user.firstname.isNotNull(), new PageRequest(0, 10, new Sort( + Sort.Direction.ASC, "manager.firstname"))); + + assertThat(page.getContent(), hasSize(3)); + assertThat(page.getContent(), hasItems(dave, oliver, carter)); + } + + /** + * @see DATAJPA-427 + */ + @Test + public void findBySpecificationWithSortBySingularPropertyInPageableShouldUseSortNullValuesFirst() { + + QUser user = QUser.user; + + Page page = repository.findAll(user.firstname.isNotNull(), new PageRequest(0, 10, new Sort( + Sort.Direction.ASC, "firstname"))); + + assertThat(page.getContent(), hasSize(3)); + assertThat(page.getContent(), hasItems(carter, dave, oliver)); + } + + /** + * @see DATAJPA-427 + */ + @Test + public void findBySpecificationWithSortByOrderIgnoreCaseBySingularPropertyInPageableShouldUseSortNullValuesFirst() { + + QUser user = QUser.user; + + Page page = repository.findAll(user.firstname.isNotNull(), new PageRequest(0, 10, new Sort(new Order( + Sort.Direction.ASC, "firstname").ignoreCase()))); + + assertThat(page.getContent(), hasSize(3)); + assertThat(page.getContent(), hasItems(carter, dave, oliver)); + } + + /** + * @see DATAJPA-427 + */ + @Test + public void findBySpecificationWithSortByNestedEmbeddedPropertyInPageableShouldUseSortNullValuesFirst() { + + oliver.setAddress(new Address("Germany", "Saarbrücken", "HaveItYourWay", "123")); + + QUser user = QUser.user; + + Page page = repository.findAll(user.firstname.isNotNull(), new PageRequest(0, 10, new Sort( + Sort.Direction.ASC, "address.streetName"))); + + assertThat(page.getContent(), hasSize(3)); + assertThat(page.getContent(), hasItems(dave, carter, oliver)); + assertThat(page.getContent().get(2), is(oliver)); + } } diff --git a/src/test/resources/META-INF/persistence.xml b/src/test/resources/META-INF/persistence.xml index 2c107f047..1f8dad143 100644 --- a/src/test/resources/META-INF/persistence.xml +++ b/src/test/resources/META-INF/persistence.xml @@ -26,6 +26,7 @@ org.springframework.data.jpa.domain.sample.EmbeddedIdExampleDepartment org.springframework.data.jpa.domain.sample.Customer org.springframework.data.jpa.domain.sample.Order + org.springframework.data.jpa.domain.sample.Address true