diff --git a/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java b/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java index b176250de..4405e024d 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2011 the original author or authors. + * Copyright 2008-2012 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. @@ -18,9 +18,11 @@ package org.springframework.data.jpa.repository.query; import static java.util.regex.Pattern.*; import java.util.ArrayList; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Locale; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -38,6 +40,7 @@ import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Order; import org.springframework.data.mapping.PropertyPath; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; /** * Simple utility class to create JPA queries. @@ -60,6 +63,9 @@ public abstract class QueryUtils { private static final String IDENTIFIER = "[\\p{Alnum}._$]+"; private static final String IDENTIFIER_GROUP = String.format("(%s)", IDENTIFIER); + private static final String LEFT_JOIN = "left (outer )?join " + IDENTIFIER + " (as )?" + IDENTIFIER_GROUP; + private static final Pattern LEFT_JOIN_PATTERN = Pattern.compile(LEFT_JOIN, Pattern.CASE_INSENSITIVE); + static { StringBuilder builder = new StringBuilder(); @@ -128,24 +134,76 @@ public abstract class QueryUtils { Assert.hasText(query); - if (null == sort) { + if (null == sort || !sort.iterator().hasNext()) { return query; } StringBuilder builder = new StringBuilder(query); - builder.append(" order by"); - for (Order order : sort) { - builder.append(String.format(" %s.%s %s,", alias, order.getProperty(), toJpaDirection(order))); + if (!query.contains("order by")) { + builder.append(" order by "); + } else { + builder.append(", "); } - builder.deleteCharAt(builder.length() - 1); + Set aliases = getOuterJoinAliases(query); + + for (Order order : sort) { + builder.append(getOrderClause(aliases, alias, order)); + } + + builder.delete(builder.length() - 2, builder.length()); return builder.toString(); } - public static String toJpaDirection(Order order) { + /** + * Returns the order clause for the given {@link Order}. Will prefix the clause with the given alias if the referenced + * property refers to a join alias. + * + * @param joinAliases the join aliases of the original query. + * @param alias the alias for the root entity. + * @param order the order object to build the clause for. + * @return + */ + private static String getOrderClause(Set joinAliases, String alias, Order order) { + String property = order.getProperty(); + boolean qualifyReference = true; + + for (String joinAlias : joinAliases) { + if (property.startsWith(joinAlias)) { + qualifyReference = false; + break; + } + } + + return String.format("%s%s %s, ", qualifyReference ? alias + "." : "", property, toJpaDirection(order)); + } + + /** + * Returns the aliases used for {@code left (outer) join}s. + * + * @param query + * @return + */ + static Set getOuterJoinAliases(String query) { + + Set result = new HashSet(); + Matcher matcher = LEFT_JOIN_PATTERN.matcher(query); + + while (matcher.find()) { + + String alias = matcher.group(3); + if (StringUtils.hasText(alias)) { + result.add(alias); + } + } + + return result; + } + + private static String toJpaDirection(Order order) { return order.getDirection().name().toLowerCase(Locale.US); } 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 d8c7c8078..90571a87e 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -921,6 +921,19 @@ public class UserRepositoryTests { assertThat(all.getContent().isEmpty(), is(false)); } + /** + * @see DATAJPA-252 + */ + @Test + public void bindsSortingToOuterJoinCorrectly() { + + flushTestUsers(); + + // Managers not set, make sure adding the sort does not rule out those Users + Page result = repository.findAllPaged(new PageRequest(0, 10, new Sort("manager.lastname"))); + assertThat(result.getContent(), hasSize((int) repository.count())); + } + private Page executeSpecWithSort(Sort sort) { flushTestUsers(); diff --git a/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java index 8832da60b..2c144ca09 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2011 the original author or authors. + * Copyright 2008-2012 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,12 +15,15 @@ */ package org.springframework.data.jpa.repository.query; -import static org.hamcrest.CoreMatchers.*; +import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; import static org.springframework.data.jpa.repository.query.QueryUtils.*; +import java.util.Set; + import org.hamcrest.Matcher; import org.junit.Test; +import org.springframework.data.domain.Sort; /** * Unit test for {@link QueryUtils}. @@ -131,8 +134,52 @@ public class QueryUtilsUnitTests { assertCountQuery(FQ_QUERY, "select count(u) from org.acme.domain.User$Foo_Bar u"); } - private void assertCountQuery(String originalQuery, String countQuery) { + /** + * @see DATAJPA-252 + */ + @Test + public void detectsJoinAliasesCorrectly() { + Set aliases = getOuterJoinAliases("select p from Person p left outer join x.foo b2_$ar where …"); + assertThat(aliases, hasSize(1)); + assertThat(aliases, hasItems("b2_$ar")); + + aliases = getOuterJoinAliases("select p from Person p left join x.foo b2_$ar where …"); + assertThat(aliases, hasSize(1)); + assertThat(aliases, hasItems("b2_$ar")); + + aliases = getOuterJoinAliases("select p from Person p left outer join x.foo as b2_$ar, left join x.bar as foo where …"); + assertThat(aliases, hasSize(2)); + assertThat(aliases, hasItems("b2_$ar", "foo")); + + aliases = getOuterJoinAliases("select p from Person p left join x.foo as b2_$ar, left outer join x.bar foo where …"); + assertThat(aliases, hasSize(2)); + assertThat(aliases, hasItems("b2_$ar", "foo")); + } + + /** + * @see DATAJPA-252 + */ + @Test + public void doesNotPrefixOrderReferenceIfOuterJoinAliasDetected() { + + String query = "select p from Person p left join p.address address"; + assertThat(applySorting(query, new Sort("address.city")), endsWith("order by address.city asc")); + assertThat(applySorting(query, new Sort("address.city", "lastname"), "p"), + endsWith("order by address.city asc, p.lastname asc")); + } + + /** + * @see DATAJPA-252 + */ + @Test + public void extendsExistingOrderByClausesCorrectly() { + + String query = "select p from Person p order by p.lastname asc"; + assertThat(applySorting(query, new Sort("firstname"), "p"), endsWith("order by p.lastname asc, p.firstname asc")); + } + + private void assertCountQuery(String originalQuery, String countQuery) { assertThat(createCountQueryFor(originalQuery), is(countQuery)); } } 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 6bef5f0be..af36e4650 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 @@ -1,5 +1,5 @@ /* - * Copyright 2008-2011 the original author or authors. + * Copyright 2008-2012 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. @@ -68,7 +68,7 @@ public interface UserRepository extends JpaRepository, JpaSpecifi */ User findByEmailAddress(String emailAddress); - @Query("select u from User u ") + @Query("select u from User u left outer join u.manager as manager") Page findAllPaged(Pageable pageable); /**