This is an automated email from the git hooks/post-receive script. New commit to branch feature/orderBy in repository topia. See http://git.nuiton.org/topia.git commit c6337c005da47a93a07b41bb923f81204de81377 Author: Arnaud Thimel <thimel@codelutin.com> Date: Thu Sep 11 18:04:27 2014 +0200 refs #3508 findPage(...) should fail if no 'order by' is specified --- .../it/legacy/evo3396/FetchPropertiesTest.java | 3 +- .../topia/persistence/internal/TopiaDaoTest.java | 33 ++++++++++++++-------- .../persistence/QueryMissingOrderException.java | 13 +++++++++ .../persistence/internal/AbstractTopiaDao.java | 25 +++++++++++----- 4 files changed, 55 insertions(+), 19 deletions(-) diff --git a/topia-it/src/test/java/org/nuiton/topia/it/legacy/evo3396/FetchPropertiesTest.java b/topia-it/src/test/java/org/nuiton/topia/it/legacy/evo3396/FetchPropertiesTest.java index a34b032..7eafb1c 100644 --- a/topia-it/src/test/java/org/nuiton/topia/it/legacy/evo3396/FetchPropertiesTest.java +++ b/topia-it/src/test/java/org/nuiton/topia/it/legacy/evo3396/FetchPropertiesTest.java @@ -275,7 +275,7 @@ public class FetchPropertiesTest extends AbstractLegacyTest { PaginationResult<Company> companies = companyDao .forNameIn(Arrays.asList("Code Lutin", "Cogitec")) .addFetch(Company.PROPERTY_DEPARTMENT) - .findPage(PaginationParameter.of(0, 1)); + .findPage(PaginationParameter.of(0, 1, Company.PROPERTY_NAME, false)); Assert.assertEquals(2, companies.getCount()); Assert.assertEquals(2, companies.getPageCount()); @@ -382,6 +382,7 @@ public class FetchPropertiesTest extends AbstractLegacyTest { PaginationResult<Company> companies = companyDao .forNameEquals("azetryu") .addFetch(Company.PROPERTY_DEPARTMENT) + .setOrderByArguments(Company.PROPERTY_NAME) .findPage(PaginationParameter.of(0, 5)); Assert.assertEquals(0, companies.getCount()); diff --git a/topia-it/src/test/java/org/nuiton/topia/persistence/internal/TopiaDaoTest.java b/topia-it/src/test/java/org/nuiton/topia/persistence/internal/TopiaDaoTest.java index 127679c..32d9387 100644 --- a/topia-it/src/test/java/org/nuiton/topia/persistence/internal/TopiaDaoTest.java +++ b/topia-it/src/test/java/org/nuiton/topia/persistence/internal/TopiaDaoTest.java @@ -38,6 +38,7 @@ import org.nuiton.topia.it.legacy.TopiaItLegacyDatabase; import org.nuiton.topia.it.legacy.TopiaItLegacyTopiaPersistenceContext; import org.nuiton.topia.it.legacy.test.entities.Person; import org.nuiton.topia.it.legacy.test.entities.PersonTopiaDao; +import org.nuiton.topia.persistence.QueryMissingOrderException; import org.nuiton.topia.persistence.TopiaDao; import org.nuiton.topia.persistence.TopiaEntities; import org.nuiton.topia.persistence.TopiaException; @@ -132,7 +133,7 @@ public class TopiaDaoTest { String query = "SELECT DISTINCT " + Person.PROPERTY_NAME + " " + - dao.newFromClause() + " ORDER BY " + Person.PROPERTY_NAME; + dao.newFromClause() + " ORDER BY " + Person.PROPERTY_NAME; Iterable<String> names = dao.findAllLazy(query, new HashMap<String, Object>()); Assert.assertEquals(5, Iterables.size(names)); } @@ -161,7 +162,7 @@ public class TopiaDaoTest { String query = "SELECT " + Person.PROPERTY_NAME + " " + - dao.newFromClause() + " ORDER BY " + Person.PROPERTY_NAME; + dao.newFromClause() + " ORDER BY " + Person.PROPERTY_NAME; PaginationResult<Person> pager = dao.initPagination(query, new HashMap<String, Object>(), 3); Assert.assertEquals(10, pager.getCount()); @@ -182,7 +183,7 @@ public class TopiaDaoTest { String query = "SELECT " + Person.PROPERTY_NAME + ", " + Person.PROPERTY_FIRSTNAME + " " + - dao.newFromClause() + " ORDER BY " + Person.PROPERTY_NAME; + dao.newFromClause() + " ORDER BY " + Person.PROPERTY_NAME; Iterable<String[]> names = dao.findAllLazy(query, new HashMap<String, Object>()); Assert.assertEquals(10, Iterables.size(names)); @@ -193,7 +194,7 @@ public class TopiaDaoTest { String query = "SELECT DISTINCT (" + Person.PROPERTY_NAME + ") " + - dao.newFromClause() + " ORDER BY " + Person.PROPERTY_NAME; + dao.newFromClause() + " ORDER BY " + Person.PROPERTY_NAME; dao.initPagination(query, new HashMap<String, Object>(), 3); } @@ -350,14 +351,14 @@ public class TopiaDaoTest { Assert.assertEquals(17, excepted.size()); { - PaginationParameter page = PaginationParameter.of(0, -1); + PaginationParameter page = PaginationParameter.of(0, -1, Person.PROPERTY_TOPIA_ID, false); List<Person> list = dao.find(dao.newFromClause(), new HashMap<String, Object>(), page); Assert.assertEquals(17, list.size()); } Set<String> alreadyLoaded = Sets.newHashSet(); { - PaginationParameter page = PaginationParameter.of(0, 6); + PaginationParameter page = PaginationParameter.of(0, 6, Person.PROPERTY_TOPIA_ID, false); List<Person> list = dao.find(dao.newFromClause(), new HashMap<String, Object>(), page); Assert.assertEquals(6, list.size()); @@ -368,7 +369,7 @@ public class TopiaDaoTest { } { - PaginationParameter page = PaginationParameter.of(1, 6); + PaginationParameter page = PaginationParameter.of(1, 6, Person.PROPERTY_TOPIA_ID, false); List<Person> list = dao.find(dao.newFromClause(), new HashMap<String, Object>(), page); Assert.assertEquals(6, list.size()); @@ -379,7 +380,7 @@ public class TopiaDaoTest { } { - PaginationParameter page = PaginationParameter.of(2, 6); + PaginationParameter page = PaginationParameter.of(2, 6, Person.PROPERTY_TOPIA_ID, false); List<Person> list = dao.find(dao.newFromClause(), new HashMap<String, Object>(), page); Assert.assertEquals(5, list.size()); @@ -445,7 +446,7 @@ public class TopiaDaoTest { createPersons(19); - String hql = dao.newFromClause("p") + " WHERE p." + Person.PROPERTY_NAME + " LIKE 'toto%' "; + String hql = dao.newFromClause("p") + " WHERE p." + Person.PROPERTY_NAME + " LIKE 'toto%' ORDER BY id "; PaginationResult<Person> page = dao.forHql(hql).findPage(PaginationParameter.of(0, 5)); Assert.assertEquals(5, page.getElements().size()); @@ -541,7 +542,7 @@ public class TopiaDaoTest { public void testFindPageNoData() throws TopiaException { String hql = dao.newFromClause("p"); - PaginationResult<Person> page = dao.forHql(hql).findPage(PaginationParameter.of(0, 5)); + PaginationResult<Person> page = dao.forHql(hql).findPage(PaginationParameter.of(0, 5, Person.PROPERTY_NAME, false)); Assert.assertEquals(0, page.getElements().size()); Assert.assertEquals(0, page.getCount()); @@ -550,4 +551,14 @@ public class TopiaDaoTest { } -} + + @Test (expected = QueryMissingOrderException.class) + public void testFindPageNoOrder() throws TopiaException { + + String hql = dao.newFromClause("p"); + PaginationParameter page = PaginationParameter.of(0, 5); + dao.forHql(hql).findPage(page); + + } + +} \ No newline at end of file diff --git a/topia-persistence/src/main/java/org/nuiton/topia/persistence/QueryMissingOrderException.java b/topia-persistence/src/main/java/org/nuiton/topia/persistence/QueryMissingOrderException.java index 8d58a30..b6a4bb8 100644 --- a/topia-persistence/src/main/java/org/nuiton/topia/persistence/QueryMissingOrderException.java +++ b/topia-persistence/src/main/java/org/nuiton/topia/persistence/QueryMissingOrderException.java @@ -26,6 +26,8 @@ package org.nuiton.topia.persistence; import java.util.Map; +import org.nuiton.util.pagination.PaginationParameter; + /** * If you get this exception, it means that you asked ToPIA to make an operation that needs a deterministic way to sort * the result but you didn't defined such query. @@ -42,8 +44,19 @@ public class QueryMissingOrderException extends TopiaQueryException { protected static final String MESSAGE = "Given query needs an ORDER BY clause since the API call you're using " + "needs the results sorting to be deterministic"; + protected PaginationParameter paginationParameter; + public QueryMissingOrderException(String hql, Map<String, Object> hqlParameters) { super(MESSAGE, hql, hqlParameters); } + public QueryMissingOrderException(String hql, Map<String, Object> hqlParameters, PaginationParameter paginationParameter) { + super(MESSAGE, hql, hqlParameters); + this.paginationParameter = paginationParameter; + } + + public PaginationParameter getPaginationParameter() { + return paginationParameter; + } + } diff --git a/topia-persistence/src/main/java/org/nuiton/topia/persistence/internal/AbstractTopiaDao.java b/topia-persistence/src/main/java/org/nuiton/topia/persistence/internal/AbstractTopiaDao.java index 3d4a074..1265347 100644 --- a/topia-persistence/src/main/java/org/nuiton/topia/persistence/internal/AbstractTopiaDao.java +++ b/topia-persistence/src/main/java/org/nuiton/topia/persistence/internal/AbstractTopiaDao.java @@ -581,12 +581,23 @@ public abstract class AbstractTopiaDao<E extends TopiaEntity> implements TopiaDa Preconditions.checkNotNull(hqlParameters); Preconditions.checkNotNull(page); - if (!page.getOrderClauses().isEmpty()) { + boolean hqlContainsOrderClause = hqlContainsOrderBy(hql); + boolean pageContainsOrderClause = !page.getOrderClauses().isEmpty(); - // can't have a order by clause in hql query - Preconditions.checkState( - !hqlContainsOrderBy(hql), - "An 'order by' clause was already found in hql, can't use the order of the pager"); + if (!hqlContainsOrderClause && !pageContainsOrderClause) { + throw new QueryMissingOrderException(hql, hqlParameters, page); + } + + // Must have one (and only one) order by clause in query + Preconditions.checkArgument( + hqlContainsOrderClause ^ pageContainsOrderClause, + String.format( + "One 'order by' clause (and only one) must be specified. [orderByInHql=%b] [orderByInPage=%b]", + hqlContainsOrderClause, + pageContainsOrderClause) + ); + + if (pageContainsOrderClause) { hql += " ORDER BY "; Iterable<String> orderClauses = Iterables.transform(page.getOrderClauses(), PAGINATION_ORDER_TO_HQL); @@ -595,8 +606,8 @@ public abstract class AbstractTopiaDao<E extends TopiaEntity> implements TopiaDa List<O> result = topiaJpaSupport.find( hql, - (int) page.getStartIndex(), - (int) page.getEndIndex(), + page.getStartIndex(), + page.getEndIndex(), hqlParameters); return result; -- To stop receiving notification emails like this one, please contact nuiton.org SCM administrator <admin+scm@nuiton.org>.