From da2c2fe9c382dcebd3aa4919f0c073789d817f38 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 19 Jan 2015 15:09:01 +0100 Subject: [PATCH 1/2] DATACMNS-621 - QSort treats long paths incorrectly. Prepare issue branch. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 94acb4dfff..cfbc2d49b2 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-commons - 1.10.0.BUILD-SNAPSHOT + 1.10.0.DATACMNS-621-SNAPSHOT Spring Data Core From c841f3a82e46c6bd8ef064f87dd00e06b68df522 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Tue, 20 Jan 2015 14:19:19 +0100 Subject: [PATCH 2/2] DATACMNS-621 - QSort treats long paths incorrectly. We now inspect intermediate path elements to build sort order accordingly. --- .../springframework/data/querydsl/QSort.java | 29 +++++++++-- .../data/querydsl/QSortUnitTests.java | 50 ++++++++++++++++--- .../data/querydsl/UserWrapper.java | 28 +++++++++++ .../data/querydsl/WrapperForUserWrapper.java | 28 +++++++++++ .../WrapperToWrapWrapperForUserWrapper.java | 29 +++++++++++ 5 files changed, 155 insertions(+), 9 deletions(-) create mode 100644 src/test/java/org/springframework/data/querydsl/UserWrapper.java create mode 100644 src/test/java/org/springframework/data/querydsl/WrapperForUserWrapper.java create mode 100644 src/test/java/org/springframework/data/querydsl/WrapperToWrapWrapperForUserWrapper.java diff --git a/src/main/java/org/springframework/data/querydsl/QSort.java b/src/main/java/org/springframework/data/querydsl/QSort.java index 1b8af09a60..a7c905beb0 100644 --- a/src/main/java/org/springframework/data/querydsl/QSort.java +++ b/src/main/java/org/springframework/data/querydsl/QSort.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2014 the original author or authors. + * Copyright 2013-2015 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. @@ -19,6 +19,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Stack; import org.springframework.data.domain.Sort; import org.springframework.util.Assert; @@ -31,6 +32,7 @@ * Sort option for queries that wraps a querydsl {@link OrderSpecifier}. * * @author Thomas Darimont + * @author Christoph Strobl */ public class QSort extends Sort implements Serializable { @@ -87,8 +89,8 @@ private static Order toOrder(OrderSpecifier orderSpecifier) { Assert.notNull(orderSpecifier, "Order specifier must not be null!"); Expression target = orderSpecifier.getTarget(); - Object targetElement = target instanceof Path ? ((com.mysema.query.types.Path) target).getMetadata() - .getElement() : target; + + Object targetElement = target instanceof Path ? preparePropertyPath((Path) target) : target; Assert.notNull(targetElement, "Target element must not be null!"); @@ -142,4 +144,25 @@ public QSort and(OrderSpecifier... orderSpecifiers) { Assert.notEmpty(orderSpecifiers, "OrderSpecifiers must not be null or empty!"); return and(Arrays.asList(orderSpecifiers)); } + + private static String preparePropertyPath(Path path) { + + Stack stack = new Stack(); + Path pathElement = path; + while (pathElement.getMetadata() != null && pathElement.getMetadata().getParent() != null) { + + stack.push(pathElement.getMetadata().getElement().toString()); + pathElement = pathElement.getMetadata().getParent(); + } + + StringBuilder sb = new StringBuilder(); + while (!stack.isEmpty()) { + sb.append(stack.pop()); + if (!stack.isEmpty()) { + sb.append("."); + } + } + return sb.toString(); + } + } diff --git a/src/test/java/org/springframework/data/querydsl/QSortUnitTests.java b/src/test/java/org/springframework/data/querydsl/QSortUnitTests.java index 477ab30839..841283e698 100644 --- a/src/test/java/org/springframework/data/querydsl/QSortUnitTests.java +++ b/src/test/java/org/springframework/data/querydsl/QSortUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2014 the original author or authors. + * Copyright 2013-2015 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,9 +15,8 @@ */ package org.springframework.data.querydsl; -import static org.hamcrest.Matchers.hasItems; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; import java.util.List; @@ -34,6 +33,7 @@ * * @author Thomas Darimont * @author Oliver Gierke + * @author Christoph Strobl */ public class QSortUnitTests { @@ -147,7 +147,7 @@ public void concatenatesPlainSortCorrectly() { assertThat(result, is(Matchers. iterableWithSize(2))); assertThat(result, hasItems(new Order(Direction.ASC, "lastname"), new Order(Direction.ASC, "firstname"))); } - + /** * @see DATACMNS-566 */ @@ -159,6 +159,44 @@ public void shouldSupportSortByOperatorExpressions() { Sort result = sort.and(new Sort(Direction.ASC, "lastname")); assertThat(result, is(Matchers. iterableWithSize(2))); - assertThat(result, hasItems(new Order(Direction.ASC, "lastname"), new Order(Direction.ASC, user.dateOfBirth.yearMonth().toString()))); + assertThat( + result, + hasItems(new Order(Direction.ASC, "lastname"), + new Order(Direction.ASC, user.dateOfBirth.yearMonth().toString()))); + } + + /** + * @see DATACMNS-621 + */ + @Test + public void shouldCreateSortForNestedPathCorrectly() { + + QSort sort = new QSort(QUserWrapper.userWrapper.user.firstname.asc()); + + assertThat(sort, hasItems(new Order(Direction.ASC, "user.firstname"))); + } + + /** + * @see DATACMNS-621 + */ + @Test + public void shouldCreateSortForDeepNestedPathCorrectly() { + + QSort sort = new QSort(QWrapperForUserWrapper.wrapperForUserWrapper.wrapper.user.firstname.asc()); + + assertThat(sort, hasItems(new Order(Direction.ASC, "wrapper.user.firstname"))); + } + + /** + * @see DATACMNS-621 + */ + @Test + public void shouldCreateSortForReallyDeepNestedPathCorrectly() { + + QSort sort = new QSort( + QWrapperToWrapWrapperForUserWrapper.wrapperToWrapWrapperForUserWrapper.wrapperForUserWrapper.wrapper.user.firstname + .asc()); + + assertThat(sort, hasItems(new Order(Direction.ASC, "wrapperForUserWrapper.wrapper.user.firstname"))); } } diff --git a/src/test/java/org/springframework/data/querydsl/UserWrapper.java b/src/test/java/org/springframework/data/querydsl/UserWrapper.java new file mode 100644 index 0000000000..2cb823f3cc --- /dev/null +++ b/src/test/java/org/springframework/data/querydsl/UserWrapper.java @@ -0,0 +1,28 @@ +/* + * Copyright 2015 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.querydsl; + +import com.mysema.query.annotations.QueryEntity; + +/** + * @author Christoph Strobl + */ +@QueryEntity +public class UserWrapper { + + User user; + +} diff --git a/src/test/java/org/springframework/data/querydsl/WrapperForUserWrapper.java b/src/test/java/org/springframework/data/querydsl/WrapperForUserWrapper.java new file mode 100644 index 0000000000..b9f33249bb --- /dev/null +++ b/src/test/java/org/springframework/data/querydsl/WrapperForUserWrapper.java @@ -0,0 +1,28 @@ +/* + * Copyright 2015 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.querydsl; + +import com.mysema.query.annotations.QueryEntity; + +/** + * @author Christoph Strobl + */ +@QueryEntity +public class WrapperForUserWrapper { + + UserWrapper wrapper; + +} diff --git a/src/test/java/org/springframework/data/querydsl/WrapperToWrapWrapperForUserWrapper.java b/src/test/java/org/springframework/data/querydsl/WrapperToWrapWrapperForUserWrapper.java new file mode 100644 index 0000000000..f089b01baf --- /dev/null +++ b/src/test/java/org/springframework/data/querydsl/WrapperToWrapWrapperForUserWrapper.java @@ -0,0 +1,29 @@ +/* + * Copyright 2015 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.querydsl; + +import com.mysema.query.annotations.QueryEntity; +import com.mysema.query.annotations.QueryInit; + +/** + * @author Christoph Strobl + */ +@QueryEntity +public class WrapperToWrapWrapperForUserWrapper { + + @QueryInit("wrapper.user")// + WrapperForUserWrapper wrapperForUserWrapper; +}