From 01325e62e299ae466cb86c5b475546df1a6f3add Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 14 Sep 2016 11:34:26 +0200 Subject: [PATCH] DATAJPA-965 - Polishing. Removed some unnecessary overrides in JpaOrder. --- .../data/jpa/domain/JpaSort.java | 36 ++++++------------- .../data/jpa/repository/query/QueryUtils.java | 12 ++++--- .../data/jpa/domain/JpaSortTests.java | 1 - .../repository/query/QueryUtilsUnitTests.java | 2 -- 4 files changed, 17 insertions(+), 34 deletions(-) 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 53fd2e8ab..54cd326b7 100644 --- a/src/main/java/org/springframework/data/jpa/domain/JpaSort.java +++ b/src/main/java/org/springframework/data/jpa/domain/JpaSort.java @@ -241,6 +241,7 @@ public class JpaSort extends Sort { Assert.notEmpty(properties, "Properties must not be empty!"); List orders = new ArrayList(); + for (String property : properties) { orders.add(new JpaOrder(direction, property)); } @@ -309,10 +310,17 @@ public class JpaSort extends Sort { } /** + * Custom {@link Order} that keeps a flag to indicate unsafe property handling, i.e. the String provided is not + * necessarily a property but can be an arbitrary expression piped into the query execution. We also keep an + * additional {@code ignoreCase} flag around as the constructor of the superclass is private currently. + * * @author Christoph Strobl + * @author Oliver Gierke */ public static class JpaOrder extends Order { + private static final long serialVersionUID = 1L; + private final boolean unsafe; private final boolean ignoreCase; @@ -365,32 +373,6 @@ public class JpaSort extends Sort { 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. * @@ -403,9 +385,11 @@ public class JpaSort extends Sort { 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); } 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 f40c04262..e9542e539 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 @@ -75,7 +75,6 @@ public abstract class QueryUtils { public static final String COUNT_QUERY_STRING = "select count(%s) from %s x"; public static final String DELETE_ALL_QUERY_STRING = "delete from %s x"; - private static final String DEFAULT_ALIAS = "x"; private static final String COUNT_REPLACEMENT_TEMPLATE = "select count(%s) $5$6$7"; private static final String SIMPLE_COUNT_VALUE = "$2"; private static final String COMPLEX_COUNT_VALUE = "$3$6"; @@ -109,6 +108,10 @@ public abstract class QueryUtils { private static final String FUNCTION_ALIAS_GROUP_NAME = "alias"; private static final Pattern FUNCTION_PATTERN; + private static final String UNSAFE_PROPERTY_REFERENCE = "Sort expression '%s' must only contain property references or " + + "aliases used in the select clause. If you really want to use something other than that for sorting, please use " + + "JpaSort.unsafe(…)!"; + static { StringBuilder builder = new StringBuilder(); @@ -315,7 +318,7 @@ public abstract class QueryUtils { * @param query * @return */ - static Set getFunctionAliases(String query) { + private static Set getFunctionAliases(String query) { Set result = new HashSet(); Matcher matcher = FUNCTION_PATTERN.matcher(query); @@ -323,6 +326,7 @@ public abstract class QueryUtils { while (matcher.find()) { String alias = matcher.group(FUNCTION_ALIAS_GROUP_NAME); + if (StringUtils.hasText(alias)) { result.add(alias); } @@ -675,9 +679,7 @@ public abstract class QueryUtils { } 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)); + throw new InvalidDataAccessApiUsageException(String.format(UNSAFE_PROPERTY_REFERENCE, 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 49d28e10a..3de91829a 100644 --- a/src/test/java/org/springframework/data/jpa/domain/JpaSortTests.java +++ b/src/test/java/org/springframework/data/jpa/domain/JpaSortTests.java @@ -226,5 +226,4 @@ public class JpaSortTests { 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 7d4f2334d..c59a52b1f 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 @@ -48,9 +48,7 @@ public class QueryUtilsUnitTests { @Test public void createsCountQueryCorrectly() throws Exception { - assertCountQuery(QUERY, COUNT_QUERY); - } /**