diff --git a/src/main/asciidoc/jpa.adoc b/src/main/asciidoc/jpa.adoc index d4e9d5816..c2d2965f6 100644 --- a/src/main/asciidoc/jpa.adoc +++ b/src/main/asciidoc/jpa.adoc @@ -303,6 +303,42 @@ public interface UserRepository extends JpaRepository { This also works with named native queries by adding the suffix `.count` to a copy of your query. Be aware that you probably must register a result set mapping for your count query, though. +[[jpa.query-methods.sorting]] +=== Using Sort + +Sorting can be done be either providing a `PageRequest` or using `Sort` directly. The properties actually used within the `Order` instances of `Sort` need to match to your domain model, which means they need to resolve to either a property or an alias used within the query. The JPQL defines this as a _state_field_path_expression_. + +[NOTE] +==== +Using any non referenceable path expression leads to an Exception. +==== + +Using `Sort` together with <> however allows you to sneak in non path checked `Order` instances containing _functions_ within the `ORDER BY` clause. This is possible because the `Order` is just appended to the given query string. By default we will reject any `Order` instance containing function calls, but you can use `JpaSort.unsafe` to add potentially unsafe ordering. + +.Using Sort and JpaSort +==== +[source, java] +---- +public interface UserRepository extends JpaRepository { + + @Query("select u from User u where u.lastname like ?1%") + List findByAndSort(String lastname, Sort sort); + + @Query("select u.id, LENGTH(u.firstname) as fn_len from User u where u.lastname like ?1%") + List findByAsArrayAndSort(String lastname, Sort sort); +} + +repo.findByAndSort("lannister", new Sort("firstname")); <1> +repo.findByAndSort("stark", new Sort("LENGTH(firstname)")); <2> +repo.findByAndSort("targaryen", JpaSort.unsafe("LENGTH(firstname)")); <3> +repo.findByAsArrayAndSort("bolton", new Sort("fn_len")); <4> +---- +<1> Valid `Sort` expression pointing to property in domain model. +<2> Invalid `Sort` containing function call. Thows Exception. +<3> Valid `Sort` containing explicitly _unsafe_ `Order`. +<4> Valid `Sort` expression pointing to aliased function. +==== + [[jpa.named-parameters]] === Using named parameters diff --git a/src/main/java/org/springframework/data/jpa/domain/JpaSort.java b/src/main/java/org/springframework/data/jpa/domain/JpaSort.java index 9bbc13695..53fd2e8ab 100644 --- a/src/main/java/org/springframework/data/jpa/domain/JpaSort.java +++ b/src/main/java/org/springframework/data/jpa/domain/JpaSort.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2015 the original author or authors. + * Copyright 2013-2016 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. @@ -32,6 +32,7 @@ import org.springframework.util.Assert; * * @author Thomas Darimont * @author Oliver Gierke + * @author Christoph Strobl */ public class JpaSort extends Sort { @@ -83,6 +84,10 @@ public class JpaSort extends Sort { super(combine(orders, direction, paths)); } + private JpaSort(List orders) { + super(orders); + } + /** * Returns a new {@link JpaSort} with the given sorting criteria added to the current one. * @@ -117,6 +122,30 @@ public class JpaSort extends Sort { return new JpaSort(existing, direction, Arrays.asList(paths)); } + /** + * Returns a new {@link JpaSort} with the given sorting criteria added to the current one. + * + * @param direction can be {@literal null}. + * @param properties must not be {@literal null} or empty. + * @return + */ + public JpaSort andUnsafe(Direction direction, String... properties) { + + Assert.notEmpty(properties, "Properties must not be null!"); + + List orders = new ArrayList(); + + for (Order order : this) { + orders.add(order); + } + + for (String property : properties) { + orders.add(new JpaOrder(direction, property)); + } + + return new JpaSort(orders, direction, Collections.> emptyList()); + } + /** * Turns the given {@link Attribute}s into {@link Path}s. * @@ -174,6 +203,51 @@ public class JpaSort extends Sort { return new Path(Arrays.asList(attribute)); } + /** + * Creates new unsafe {@link JpaSort} based on given properties. + * + * @param properties must not be {@literal null} or empty. + * @return + */ + public static JpaSort unsafe(String... properties) { + return unsafe(Sort.DEFAULT_DIRECTION, properties); + } + + /** + * Creates new unsafe {@link JpaSort} based on given {@link Direction} and properties. + * + * @param direction must not be {@literal null}. + * @param properties must not be {@literal null} or empty. + * @return + */ + public static JpaSort unsafe(Direction direction, String... properties) { + + Assert.notNull(direction, "Direction must not be null!"); + Assert.notEmpty(properties, "Properties must not be empty!"); + Assert.noNullElements(properties, "Properties must not contain null values!"); + + return unsafe(direction, Arrays.asList(properties)); + } + + /** + * Creates new unsafe {@link JpaSort} based on given {@link Direction} and properties. + * + * @param direction must not be {@literal null}. + * @param properties must not be {@literal null} or empty. + * @return + */ + public static JpaSort unsafe(Direction direction, List properties) { + + Assert.notEmpty(properties, "Properties must not be empty!"); + + List orders = new ArrayList(); + for (String property : properties) { + orders.add(new JpaOrder(direction, property)); + } + + return new JpaSort(orders); + } + /** * Value object to abstract a collection of {@link Attribute}s. * @@ -233,4 +307,131 @@ public class JpaSort extends Sort { return builder.length() == 0 ? "" : builder.substring(0, builder.lastIndexOf(".")); } } + + /** + * @author Christoph Strobl + */ + public static class JpaOrder extends Order { + + private final boolean unsafe; + private final boolean ignoreCase; + + /** + * Creates a new {@link JpaOrder} instance. if order is {@literal null} then order defaults to + * {@link Sort#DEFAULT_DIRECTION} + * + * @param direction can be {@literal null}, will default to {@link Sort#DEFAULT_DIRECTION}. + * @param property must not be {@literal null}. + */ + private JpaOrder(Direction direction, String property) { + this(direction, property, NullHandling.NATIVE); + } + + /** + * Creates a new {@link Order} instance. if order is {@literal null} then order defaults to + * {@link Sort#DEFAULT_DIRECTION}. + * + * @param direction can be {@literal null}, will default to {@link Sort#DEFAULT_DIRECTION}. + * @param property must not be {@literal null}. + * @param nullHandlingHint can be {@literal null}, will default to {@link NullHandling#NATIVE}. + */ + private JpaOrder(Direction direction, String property, NullHandling nullHandlingHint) { + this(direction, property, nullHandlingHint, false, true); + } + + private JpaOrder(Direction direction, String property, NullHandling nullHandling, boolean ignoreCase, + boolean unsafe) { + + super(direction, property, nullHandling); + this.ignoreCase = ignoreCase; + this.unsafe = unsafe; + } + + /* + * (non-Javadoc) + * @see org.springframework.data.domain.Sort.Order#with(org.springframework.data.domain.Sort.Direction) + */ + @Override + public JpaOrder with(Direction order) { + return new JpaOrder(order, getProperty(), getNullHandling(), isIgnoreCase(), this.unsafe); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.domain.Sort.Order#with(org.springframework.data.domain.Sort.NullHandling) + */ + @Override + public JpaOrder with(NullHandling nullHandling) { + return new JpaOrder(getDirection(), getProperty(), nullHandling, isIgnoreCase(), this.unsafe); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.domain.Sort.Order#nullsFirst() + */ + @Override + public JpaOrder nullsFirst() { + return with(NullHandling.NULLS_FIRST); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.domain.Sort.Order#nullsLast() + */ + @Override + public JpaOrder nullsLast() { + return with(NullHandling.NULLS_LAST); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.domain.Sort.Order#nullsNative() + */ + public JpaOrder nullsNative() { + return with(NullHandling.NATIVE); + } + + /** + * Creates new {@link Sort} with potentially unsafe {@link Order} instances. + * + * @param properties must not be {@literal null}. + * @return + */ + public Sort withUnsafe(String... properties) { + + Assert.notEmpty(properties, "Properties must not be empty!"); + Assert.noNullElements(properties, "Properties must not contain null values!"); + + List orders = new ArrayList(); + for (String property : properties) { + orders.add(new JpaOrder(getDirection(), property, getNullHandling(), isIgnoreCase(), this.unsafe)); + } + return new Sort(orders); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.domain.Sort.Order#ignoreCase() + */ + @Override + public JpaOrder ignoreCase() { + return new JpaOrder(getDirection(), getProperty(), getNullHandling(), true, this.unsafe); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.domain.Sort.Order#isIgnoreCase() + */ + @Override + public boolean isIgnoreCase() { + return super.isIgnoreCase() || ignoreCase; + } + + /** + * @return true if {@link JpaOrder} created {@link #withUnsafe(String...)}. + */ + public boolean isUnsafe() { + return unsafe; + } + } } 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 3825b10dd..f40c04262 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 @@ -53,8 +53,10 @@ import javax.persistence.metamodel.ManagedType; import javax.persistence.metamodel.PluralAttribute; import org.springframework.core.annotation.AnnotationUtils; +import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Order; +import org.springframework.data.jpa.domain.JpaSort.JpaOrder; import org.springframework.data.mapping.PropertyPath; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -66,6 +68,7 @@ import org.springframework.util.StringUtils; * @author Kevin Raymond * @author Thomas Darimont * @author Komi Innocent + * @author Christoph Strobl */ public abstract class QueryUtils { @@ -102,6 +105,10 @@ public abstract class QueryUtils { private static final int QUERY_JOIN_ALIAS_GROUP_INDEX = 2; private static final int VARIABLE_NAME_GROUP_INDEX = 4; + private static final Pattern PUNCTATION_PATTERN = Pattern.compile(".*((?![\\._])[\\p{Punct}|\\s])"); + private static final String FUNCTION_ALIAS_GROUP_NAME = "alias"; + private static final Pattern FUNCTION_PATTERN; + static { StringBuilder builder = new StringBuilder(); @@ -145,6 +152,13 @@ public abstract class QueryUtils { builder.append("\\)"); CONSTRUCTOR_EXPRESSION = compile(builder.toString(), CASE_INSENSITIVE + DOTALL); + + builder = new StringBuilder(); + builder.append("\\s+"); // at least one space + builder.append("\\w+\\([0-9a-zA-z\\._,\\s']+\\)"); // any function call including parameters within the brackets + builder.append("\\s+[as|AS]+\\s+(?<" + FUNCTION_ALIAS_GROUP_NAME + ">[\\w\\.]+)"); // the potential alias + + FUNCTION_PATTERN = compile(builder.toString()); } /** @@ -227,9 +241,10 @@ public abstract class QueryUtils { } Set aliases = getOuterJoinAliases(query); + Set functionAliases = getFunctionAliases(query); for (Order order : sort) { - builder.append(getOrderClause(aliases, alias, order)).append(", "); + builder.append(getOrderClause(aliases, functionAliases, alias, order)).append(", "); } builder.delete(builder.length() - 2, builder.length()); @@ -246,9 +261,16 @@ public abstract class QueryUtils { * @param order the order object to build the clause for. * @return */ - private static String getOrderClause(Set joinAliases, String alias, Order order) { + private static String getOrderClause(Set joinAliases, Set functionAlias, String alias, Order order) { String property = order.getProperty(); + + checkSortExpression(order); + + if (functionAlias.contains(property)) { + return String.format("%s %s", property, toJpaDirection(order)); + } + boolean qualifyReference = !property.contains("("); // ( indicates a function for (String joinAlias : joinAliases) { @@ -287,6 +309,28 @@ public abstract class QueryUtils { return result; } + /** + * Returns the aliases used for aggregate functions like {@code SUM, COUNT, ...}. + * + * @param query + * @return + */ + static Set getFunctionAliases(String query) { + + Set result = new HashSet(); + Matcher matcher = FUNCTION_PATTERN.matcher(query); + + while (matcher.find()) { + + String alias = matcher.group(FUNCTION_ALIAS_GROUP_NAME); + if (StringUtils.hasText(alias)) { + result.add(alias); + } + } + + return result; + } + private static String toJpaDirection(Order order) { return order.getDirection().name().toLowerCase(Locale.US); } @@ -617,4 +661,23 @@ public abstract class QueryUtils { return false; } + + /** + * Check any given {@link JpaOrder#isUnsafe()} order for presence of at least one property offending the + * {@link #PUNCTATION_PATTERN} and throw an {@link Exception} indicating potential unsafe order by expression. + * + * @param order + */ + private static void checkSortExpression(Order order) { + + if (order instanceof JpaOrder && ((JpaOrder) order).isUnsafe()) { + return; + } + + if (PUNCTATION_PATTERN.matcher(order.getProperty()).find()) { + throw new InvalidDataAccessApiUsageException(String + .format("Sort expression '%s' must not contain functions or expressions. Please use JpaSort.unsafe.", order)); + } + } + } diff --git a/src/test/java/org/springframework/data/jpa/domain/JpaSortTests.java b/src/test/java/org/springframework/data/jpa/domain/JpaSortTests.java index 56db5736f..49d28e10a 100644 --- a/src/test/java/org/springframework/data/jpa/domain/JpaSortTests.java +++ b/src/test/java/org/springframework/data/jpa/domain/JpaSortTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2015 the original author or authors. + * Copyright 2013-2016 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. @@ -29,6 +29,7 @@ import org.junit.runner.RunWith; 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.JpaSort.JpaOrder; import org.springframework.data.jpa.domain.JpaSort.Path; import org.springframework.data.jpa.domain.sample.Address_; import org.springframework.data.jpa.domain.sample.MailMessage_; @@ -47,6 +48,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; * @see DATAJPA-12 * @author Thomas Darimont * @author Oliver Gierke + * @author Christoph Strobl */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration("classpath:infrastructure.xml") @@ -173,4 +175,56 @@ public class JpaSortTests { assertThat(new JpaSort(path(User_.colleagues).dot(User_.roles).dot(Role_.name)), // hasItem(new Order(ASC, "colleagues.roles.name"))); } + + /** + * @see DATAJPA-??? + */ + @Test + public void createsUnsafeSortCorrectly() { + + JpaSort sort = JpaSort.unsafe(DESC, "foo.bar"); + + assertThat(sort, hasItem(new Order(DESC, "foo.bar"))); + assertThat(sort.getOrderFor("foo.bar"), is(instanceOf(JpaOrder.class))); + } + + /** + * @see DATAJPA-??? + */ + @Test + public void createsUnsafeSortWithMultiplePropertiesCorrectly() { + + JpaSort sort = JpaSort.unsafe(DESC, "foo.bar", "spring.data"); + + assertThat(sort, hasItems(new Order(DESC, "foo.bar"), new Order(DESC, "spring.data"))); + assertThat(sort.getOrderFor("foo.bar"), is(instanceOf(JpaOrder.class))); + assertThat(sort.getOrderFor("spring.data"), is(instanceOf(JpaOrder.class))); + } + + /** + * @see DATAJPA-??? + */ + @Test + public void combinesSafeAndUnsafeSortCorrectly() { + + JpaSort sort = new JpaSort(path(User_.colleagues).dot(User_.roles).dot(Role_.name)).andUnsafe(DESC, "foo.bar"); + + assertThat(sort, hasItems(new Order(ASC, "colleagues.roles.name"), new Order(DESC, "foo.bar"))); + assertThat(sort.getOrderFor("colleagues.roles.name"), is(not(instanceOf(JpaOrder.class)))); + assertThat(sort.getOrderFor("foo.bar"), is(instanceOf(JpaOrder.class))); + } + + /** + * @see DATAJPA-??? + */ + @Test + public void combinesUnsafeAndSafeSortCorrectly() { + + Sort sort = JpaSort.unsafe(DESC, "foo.bar").and(ASC, path(User_.colleagues).dot(User_.roles).dot(Role_.name)); + + assertThat(sort, hasItems(new Order(ASC, "colleagues.roles.name"), new Order(DESC, "foo.bar"))); + assertThat(sort.getOrderFor("colleagues.roles.name"), is(not(instanceOf(JpaOrder.class)))); + assertThat(sort.getOrderFor("foo.bar"), is(instanceOf(JpaOrder.class))); + } + } 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 30ca4f28a..7d4f2334d 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-2015 the original author or authors. + * Copyright 2008-2016 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. @@ -23,7 +23,9 @@ import java.util.Set; import org.hamcrest.Matcher; import org.junit.Test; +import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.data.domain.Sort; +import org.springframework.data.jpa.domain.JpaSort; /** * Unit test for {@link QueryUtils}. @@ -31,6 +33,7 @@ import org.springframework.data.domain.Sort; * @author Oliver Gierke * @author Thomas Darimont * @author Komi Innocent + * @author Christoph Strobl */ public class QueryUtilsUnitTests { @@ -230,7 +233,7 @@ public class QueryUtilsUnitTests { /** * @see DATAJPA-148 */ - @Test + @Test(expected = InvalidDataAccessApiUsageException.class) public void doesNotPrefixSortsIfFunction() { Sort sort = new Sort("sum(foo)"); @@ -361,6 +364,134 @@ public class QueryUtilsUnitTests { endsWith("order by firstname asc")); } + /** + * @see DATAJPA-??? + */ + @Test(expected = InvalidDataAccessApiUsageException.class) + public void doesNotAllowWhitespaceInSort() { + + Sort sort = new Sort("case when foo then bar"); + applySorting("select p from Person p", sort, "p"); + } + + /** + * @see DATAJPA-??? + */ + @Test + public void doesNotPrefixUnsageJpaSortFunctionCalls() { + + JpaSort sort = JpaSort.unsafe("sum(foo)"); + assertThat(applySorting("select p from Person p", sort, "p"), endsWith("order by sum(foo) asc")); + } + + /** + * @see DATAJPA-??? + */ + @Test + public void doesNotPrefixMultipleAliasedFunctionCalls() { + + String query = "SELECT AVG(m.price) AS avgPrice, SUM(m.stocks) AS sumStocks FROM Magazine m"; + Sort sort = new Sort("avgPrice", "sumStocks"); + + assertThat(applySorting(query, sort, "m"), endsWith("order by avgPrice asc, sumStocks asc")); + } + + /** + * @see DATAJPA-??? + */ + @Test + public void doesNotPrefixSingleAliasedFunctionCalls() { + + String query = "SELECT AVG(m.price) AS avgPrice FROM Magazine m"; + Sort sort = new Sort("avgPrice"); + + assertThat(applySorting(query, sort, "m"), endsWith("order by avgPrice asc")); + } + + /** + * @see DATAJPA-??? + */ + @Test + public void prefixesSingleNonAliasedFunctionCallRelatedSortProperty() { + + String query = "SELECT AVG(m.price) AS avgPrice FROM Magazine m"; + Sort sort = new Sort("someOtherProperty"); + + assertThat(applySorting(query, sort, "m"), endsWith("order by m.someOtherProperty asc")); + } + + /** + * @see DATAJPA-??? + */ + @Test + public void prefixesNonAliasedFunctionCallRelatedSortPropertyWhenSelectClauseContainesAliasedFunctionForDifferentProperty() { + + String query = "SELECT m.name, AVG(m.price) AS avgPrice FROM Magazine m"; + Sort sort = new Sort("name", "avgPrice"); + + assertThat(applySorting(query, sort, "m"), endsWith("order by m.name asc, avgPrice asc")); + } + + /** + * @see DATAJPA-??? + */ + @Test + public void doesNotPrefixAliasedFunctionCallNameWithMultipleNumericParameters() { + + String query = "SELECT SUBSTRING(m.name, 2, 5) AS trimmedName FROM Magazine m"; + Sort sort = new Sort("trimmedName"); + + assertThat(applySorting(query, sort, "m"), endsWith("order by trimmedName asc")); + } + + /** + * @see DATAJPA-??? + */ + @Test + public void doesNotPrefixAliasedFunctionCallNameWithMultipleStringParameters() { + + String query = "SELECT CONCAT(m.name, 'foo') AS extendedName FROM Magazine m"; + Sort sort = new Sort("extendedName"); + + assertThat(applySorting(query, sort, "m"), endsWith("order by extendedName asc")); + } + + /** + * @see DATAJPA-??? + */ + @Test + public void doesNotPrefixAliasedFunctionCallNameWithUnderscores() { + + String query = "SELECT AVG(m.price) AS avg_price FROM Magazine m"; + Sort sort = new Sort("avg_price"); + + assertThat(applySorting(query, sort, "m"), endsWith("order by avg_price asc")); + } + + /** + * @see DATAJPA-??? + */ + @Test + public void doesNotPrefixAliasedFunctionCallNameWithDots() { + + String query = "SELECT AVG(m.price) AS m.avg FROM Magazine m"; + Sort sort = new Sort("m.avg"); + + assertThat(applySorting(query, sort, "m"), endsWith("order by m.avg asc")); + } + + /** + * @see DATAJPA-??? + */ + @Test + public void doesNotPrefixAliasedFunctionCallNameWhenQueryStringContainsMultipleWhiteSpaces() { + + String query = "SELECT AVG( m.price ) AS avgPrice FROM Magazine m"; + Sort sort = new Sort("avgPrice"); + + assertThat(applySorting(query, sort, "m"), endsWith("order by avgPrice asc")); + } + private static void assertCountQuery(String originalQuery, String countQuery) { assertThat(createCountQueryFor(originalQuery), is(countQuery)); }