From 1dd0f3cbf8170a3469dad8f4f8f074549bb6286d Mon Sep 17 00:00:00 2001 From: rob bygrave Date: Mon, 17 Feb 2020 14:51:00 +1300 Subject: [PATCH 1/2] #1943 - Use limit & offset via setFirstRow() and setMaxRows() on filterMany() --- .../ebeaninternal/api/SpiExpressionList.java | 7 ++++ .../expression/FilterExpressionList.java | 27 +++++++++++---- .../server/querydefn/OrmQueryProperties.java | 4 +-- .../org/tests/query/TestQueryFilterMany.java | 33 +++++++++++++++++++ 4 files changed, 63 insertions(+), 8 deletions(-) diff --git a/src/main/java/io/ebeaninternal/api/SpiExpressionList.java b/src/main/java/io/ebeaninternal/api/SpiExpressionList.java index cc07d2787..3cf5c2e64 100644 --- a/src/main/java/io/ebeaninternal/api/SpiExpressionList.java +++ b/src/main/java/io/ebeaninternal/api/SpiExpressionList.java @@ -36,4 +36,11 @@ public interface SpiExpressionList extends ExpressionList, SpiExpression { * Write the top level where expressions taking into account possible extra idEquals expression. */ void writeDocQuery(DocQueryContext context, SpiExpression idEquals) throws IOException; + + /** + * Apply firstRow maxRows limits on the filterMany query. + */ + default void applyRowLimits(SpiQuery query) { + // do nothing by default + } } diff --git a/src/main/java/io/ebeaninternal/server/expression/FilterExpressionList.java b/src/main/java/io/ebeaninternal/server/expression/FilterExpressionList.java index ffaa52702..a79354cd2 100644 --- a/src/main/java/io/ebeaninternal/server/expression/FilterExpressionList.java +++ b/src/main/java/io/ebeaninternal/server/expression/FilterExpressionList.java @@ -9,6 +9,7 @@ import io.ebean.Junction; import io.ebean.OrderBy; import io.ebean.Query; import io.ebeaninternal.api.SpiExpressionList; +import io.ebeaninternal.api.SpiQuery; import javax.persistence.PersistenceException; import java.util.Collection; @@ -25,6 +26,9 @@ public class FilterExpressionList extends DefaultExpressionList { private final FilterExprPath pathPrefix; + private int firstRow; + private int maxRows; + public FilterExpressionList(FilterExprPath pathPrefix, FilterExpressionList original) { super(null, original.expr, null, original.getUnderlyingList()); this.pathPrefix = pathPrefix; @@ -144,11 +148,6 @@ public class FilterExpressionList extends DefaultExpressionList { throw new PersistenceException(notAllowedMessage); } - @Override - public Query setFirstRow(int firstRow) { - return rootQuery.setFirstRow(firstRow); - } - @Override public Query setMapKey(String mapKey) { return rootQuery.setMapKey(mapKey); @@ -156,7 +155,14 @@ public class FilterExpressionList extends DefaultExpressionList { @Override public Query setMaxRows(int maxRows) { - return rootQuery.setMaxRows(maxRows); + this.maxRows = maxRows; + return rootQuery; + } + + @Override + public Query setFirstRow(int firstRow) { + this.firstRow = firstRow; + return rootQuery; } @Override @@ -169,5 +175,14 @@ public class FilterExpressionList extends DefaultExpressionList { return rootQuery.where(); } + @Override + public void applyRowLimits(SpiQuery query) { + if (firstRow > 0) { + query.setFirstRow(firstRow); + } + if (maxRows > 0) { + query.setMaxRows(maxRows); + } + } } diff --git a/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryProperties.java b/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryProperties.java index cf03b0188..805c3bf68 100644 --- a/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryProperties.java +++ b/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryProperties.java @@ -245,9 +245,9 @@ public class OrmQueryProperties implements Serializable { } if (filterMany != null) { + filterMany.applyRowLimits(query); SpiExpressionList trimPath = filterMany.trimPath(path.length() + 1); - List underlyingList = trimPath.getUnderlyingList(); - for (SpiExpression spiExpression : underlyingList) { + for (SpiExpression spiExpression : trimPath.getUnderlyingList()) { query.where().add(spiExpression); } } diff --git a/src/test/java/org/tests/query/TestQueryFilterMany.java b/src/test/java/org/tests/query/TestQueryFilterMany.java index fcc734a98..b8dc4ed45 100644 --- a/src/test/java/org/tests/query/TestQueryFilterMany.java +++ b/src/test/java/org/tests/query/TestQueryFilterMany.java @@ -1,7 +1,9 @@ package org.tests.query; import io.ebean.BaseTestCase; +import io.ebean.DB; import io.ebean.Ebean; +import io.ebean.ExpressionList; import io.ebean.FetchConfig; import io.ebean.Query; import org.ebeantest.LoggedSqlCollector; @@ -44,6 +46,37 @@ public class TestQueryFilterMany extends BaseTestCase { } + @Test + public void test_firstMaxRows() { + + ResetBasicData.reset(); + + LoggedSqlCollector.start(); + + final Query query = DB.find(Customer.class) + .where().ieq("name", "Rob") + .order().asc("id").setMaxRows(5); + + final ExpressionList filterMany = query.filterMany("orders").eq("status", Order.Status.NEW); + filterMany.setMaxRows(100); + filterMany.setFirstRow(3); + + final List customers = query.findList(); + assertThat(customers).isNotEmpty(); + List sqlList = LoggedSqlCollector.stop(); + assertEquals(2, sqlList.size()); + + assertThat(sqlList.get(0)).contains("lower(t0.name) = ?"); + assertThat(sqlList.get(1)).contains("status = ?"); + + if (isH2() || isPostgres()) { + assertThat(sqlList.get(0)).doesNotContain("offset"); + assertThat(sqlList.get(0)).contains(" limit 5"); + assertThat(sqlList.get(1)).contains(" offset 3"); + assertThat(sqlList.get(1)).contains(" limit 100"); + } + } + @Test public void test_with_findOne() { From e0d322d80a03f914e8bf0de230046e447e543b6c Mon Sep 17 00:00:00 2001 From: rob bygrave Date: Mon, 17 Feb 2020 17:19:27 +1300 Subject: [PATCH 2/2] #1943 - Return ExpressionList from setMaxRows() setFirstRow() to allow fluid style This is to allow fluid style use of setMaxRows() setFirstRow() with filterMany query --- src/main/java/io/ebean/ExpressionList.java | 6 +- .../expression/DefaultExpressionList.java | 15 ++--- .../expression/FilterExpressionList.java | 8 +-- .../server/expression/JunctionExpression.java | 6 +- .../java/org/tests/basic/TestLimitQuery.java | 4 +- .../org/tests/query/TestQueryFilterMany.java | 57 +++++++++++++++++++ .../org/tests/query/TestQueryFindIterate.java | 4 +- 7 files changed, 79 insertions(+), 21 deletions(-) diff --git a/src/main/java/io/ebean/ExpressionList.java b/src/main/java/io/ebean/ExpressionList.java index 653d54be7..36b4596fa 100644 --- a/src/main/java/io/ebean/ExpressionList.java +++ b/src/main/java/io/ebean/ExpressionList.java @@ -516,7 +516,7 @@ public interface ExpressionList { * @param expressions Filter expressions with and, or and ? or ?1 type bind parameters * @param params Bind parameters used in the expressions */ - Query filterMany(String manyProperty, String expressions, Object... params); + ExpressionList filterMany(String manyProperty, String expressions, Object... params); /** * Specify specific properties to fetch on the main/root bean (aka partial @@ -567,14 +567,14 @@ public interface ExpressionList { * * @see Query#setFirstRow(int) */ - Query setFirstRow(int firstRow); + ExpressionList setFirstRow(int firstRow); /** * Set the maximum number of rows to fetch. * * @see Query#setMaxRows(int) */ - Query setMaxRows(int maxRows); + ExpressionList setMaxRows(int maxRows); /** * Set the name of the property which values become the key of a map. diff --git a/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionList.java b/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionList.java index 6d5dd40b2..e4b5aadfd 100644 --- a/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionList.java +++ b/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionList.java @@ -480,9 +480,8 @@ public class DefaultExpressionList implements SpiExpressionList { } @Override - public Query filterMany(String manyProperty, String expressions, Object... params) { - query.filterMany(manyProperty).where(expressions, params); - return query; + public ExpressionList filterMany(String manyProperty, String expressions, Object... params) { + return query.filterMany(manyProperty).where(expressions, params); } @Override @@ -521,13 +520,15 @@ public class DefaultExpressionList implements SpiExpressionList { } @Override - public Query setFirstRow(int firstRow) { - return query.setFirstRow(firstRow); + public ExpressionList setFirstRow(int firstRow) { + query.setFirstRow(firstRow); + return this; } @Override - public Query setMaxRows(int maxRows) { - return query.setMaxRows(maxRows); + public ExpressionList setMaxRows(int maxRows) { + query.setMaxRows(maxRows); + return this; } @Override diff --git a/src/main/java/io/ebeaninternal/server/expression/FilterExpressionList.java b/src/main/java/io/ebeaninternal/server/expression/FilterExpressionList.java index a79354cd2..081cc9793 100644 --- a/src/main/java/io/ebeaninternal/server/expression/FilterExpressionList.java +++ b/src/main/java/io/ebeaninternal/server/expression/FilterExpressionList.java @@ -154,15 +154,15 @@ public class FilterExpressionList extends DefaultExpressionList { } @Override - public Query setMaxRows(int maxRows) { + public ExpressionList setMaxRows(int maxRows) { this.maxRows = maxRows; - return rootQuery; + return this; } @Override - public Query setFirstRow(int firstRow) { + public ExpressionList setFirstRow(int firstRow) { this.firstRow = firstRow; - return rootQuery; + return this; } @Override diff --git a/src/main/java/io/ebeaninternal/server/expression/JunctionExpression.java b/src/main/java/io/ebeaninternal/server/expression/JunctionExpression.java index 00d7b83b2..1126f04d6 100644 --- a/src/main/java/io/ebeaninternal/server/expression/JunctionExpression.java +++ b/src/main/java/io/ebeaninternal/server/expression/JunctionExpression.java @@ -337,7 +337,7 @@ class JunctionExpression implements SpiJunction, SpiExpression, Expression } @Override - public Query filterMany(String manyProperty, String expressions, Object... params) { + public ExpressionList filterMany(String manyProperty, String expressions, Object... params) { throw new IllegalStateException("filterMany not allowed on Junction expression list"); } @@ -864,7 +864,7 @@ class JunctionExpression implements SpiJunction, SpiExpression, Expression } @Override - public Query setFirstRow(int firstRow) { + public ExpressionList setFirstRow(int firstRow) { return exprList.setFirstRow(firstRow); } @@ -874,7 +874,7 @@ class JunctionExpression implements SpiJunction, SpiExpression, Expression } @Override - public Query setMaxRows(int maxRows) { + public ExpressionList setMaxRows(int maxRows) { return exprList.setMaxRows(maxRows); } diff --git a/src/test/java/org/tests/basic/TestLimitQuery.java b/src/test/java/org/tests/basic/TestLimitQuery.java index 6e92f6b61..8b6fd2bc7 100644 --- a/src/test/java/org/tests/basic/TestLimitQuery.java +++ b/src/test/java/org/tests/basic/TestLimitQuery.java @@ -53,7 +53,7 @@ public class TestLimitQuery extends BaseTestCase { .fetch("details") .where().gt("details.id", 0) .setMaxRows(3) - .setFirstRow(0); + .setFirstRow(0).query(); query.findList(); @@ -96,7 +96,7 @@ public class TestLimitQuery extends BaseTestCase { .setAutoTune(false) .fetch("details") .where().gt("details.id", 0) - .setMaxRows(10); + .setMaxRows(10).query(); //.findList(); List list = query.findList(); diff --git a/src/test/java/org/tests/query/TestQueryFilterMany.java b/src/test/java/org/tests/query/TestQueryFilterMany.java index b8dc4ed45..20627f8e6 100644 --- a/src/test/java/org/tests/query/TestQueryFilterMany.java +++ b/src/test/java/org/tests/query/TestQueryFilterMany.java @@ -46,6 +46,34 @@ public class TestQueryFilterMany extends BaseTestCase { } + @Test + public void filterMany_firstMaxRows_fluidStyle() { + + ResetBasicData.reset(); + + LoggedSqlCollector.start(); + + final Query query = DB.find(Customer.class) + .where().ieq("name", "Rob") + // fluid style adding maxRows/firstRow to filterMany + .filterMany("orders").eq("status", Order.Status.NEW).setMaxRows(100).setFirstRow(3) + .order().asc("id").setMaxRows(5); + + final List customers = query.findList(); + assertThat(customers).isNotEmpty(); + List sqlList = LoggedSqlCollector.stop(); + assertEquals(2, sqlList.size()); + assertThat(sqlList.get(0)).contains("lower(t0.name) = ?"); + assertThat(sqlList.get(1)).contains("status = ?"); + + if (isH2() || isPostgres()) { + assertThat(sqlList.get(0)).doesNotContain("offset"); + assertThat(sqlList.get(0)).contains(" limit 5"); + assertThat(sqlList.get(1)).contains(" offset 3"); + assertThat(sqlList.get(1)).contains(" limit 100"); + } + } + @Test public void test_firstMaxRows() { @@ -57,6 +85,7 @@ public class TestQueryFilterMany extends BaseTestCase { .where().ieq("name", "Rob") .order().asc("id").setMaxRows(5); + // non-fluid style adding maxRows/firstRow final ExpressionList filterMany = query.filterMany("orders").eq("status", Order.Status.NEW); filterMany.setMaxRows(100); filterMany.setFirstRow(3); @@ -77,6 +106,34 @@ public class TestQueryFilterMany extends BaseTestCase { } } + @Test + public void filterMany_firstMaxRows_expressionFluidStyle() { + + ResetBasicData.reset(); + + LoggedSqlCollector.start(); + + final Query query = DB.find(Customer.class) + .where().ieq("name", "Rob") + // use expression + fluid style adding maxRows/firstRow to filterMany + .filterMany("orders", "status = ?", Order.Status.NEW).setMaxRows(100).setFirstRow(3) + .order().asc("id").setMaxRows(5); + + final List customers = query.findList(); + assertThat(customers).isNotEmpty(); + List sqlList = LoggedSqlCollector.stop(); + assertEquals(2, sqlList.size()); + assertThat(sqlList.get(0)).contains("lower(t0.name) = ?"); + assertThat(sqlList.get(1)).contains("status = ?"); + + if (isH2() || isPostgres()) { + assertThat(sqlList.get(0)).doesNotContain("offset"); + assertThat(sqlList.get(0)).contains(" limit 5"); + assertThat(sqlList.get(1)).contains(" offset 3"); + assertThat(sqlList.get(1)).contains(" limit 100"); + } + } + @Test public void test_with_findOne() { diff --git a/src/test/java/org/tests/query/TestQueryFindIterate.java b/src/test/java/org/tests/query/TestQueryFindIterate.java index ccca68cdf..307a70d73 100644 --- a/src/test/java/org/tests/query/TestQueryFindIterate.java +++ b/src/test/java/org/tests/query/TestQueryFindIterate.java @@ -196,7 +196,7 @@ public class TestQueryFindIterate extends BaseTestCase { Query query = server.find(Customer.class) .setAutoTune(false) .where().gt("id", "JUNK_NOT_A_LONG") - .setMaxRows(2); + .setMaxRows(2).query(); // this throws an exception immediately query.findEach(bean -> { @@ -221,7 +221,7 @@ public class TestQueryFindIterate extends BaseTestCase { Query query = server.find(Customer.class) .setAutoTune(false) .where().gt("id", 0) - .setMaxRows(2); + .setMaxRows(2).query(); query.findEach(customer -> { if (customer != null) {