diff --git a/ebean-core/src/main/java/io/ebeaninternal/api/ManyWhereJoins.java b/ebean-core/src/main/java/io/ebeaninternal/api/ManyWhereJoins.java index 2278d5d11..463f1705e 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/api/ManyWhereJoins.java +++ b/ebean-core/src/main/java/io/ebeaninternal/api/ManyWhereJoins.java @@ -50,7 +50,6 @@ public final class ManyWhereJoins implements Serializable { * Add a many where join. */ public void add(ElPropertyDeploy elProp) { - String join = elProp.elPrefix(); BeanProperty p = elProp.beanProperty(); if (p instanceof BeanPropertyAssocMany) { @@ -111,7 +110,6 @@ public final class ManyWhereJoins implements Serializable { * Return the set of property names for the many where joins. */ public TreeSet getPropertyNames() { - TreeSet propertyNames = new TreeSet<>(); for (PropertyJoin join : joins.values()) { propertyNames.add(join.getProperty()); diff --git a/ebean-core/src/main/java/io/ebeaninternal/api/SpiExpression.java b/ebean-core/src/main/java/io/ebeaninternal/api/SpiExpression.java index e4392bf17..030042d96 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/api/SpiExpression.java +++ b/ebean-core/src/main/java/io/ebeaninternal/api/SpiExpression.java @@ -110,4 +110,11 @@ public interface SpiExpression extends Expression { * Apply property prefix when filterMany expressions included into main query. */ void prefixProperty(String path); + + /** + * Return a copy of the expression (as part of creating a query copy). + */ + default SpiExpression copy() { + return this; + } } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionList.java b/ebean-core/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionList.java index 26f225f3a..234fd28c2 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionList.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionList.java @@ -76,11 +76,7 @@ public class DefaultExpressionList implements SpiExpressionList { } public DefaultExpressionList(Query query, ExpressionList parentExprList) { - this(query, query.getExpressionFactory(), parentExprList); - } - - DefaultExpressionList(Query query, ExpressionFactory expr, ExpressionList parentExprList) { - this(query, expr, parentExprList, new ArrayList<>()); + this(query, query.getExpressionFactory(), parentExprList, new ArrayList<>()); } DefaultExpressionList(Query query, ExpressionFactory expr, ExpressionList parentExprList, List list) { @@ -243,13 +239,12 @@ public class DefaultExpressionList implements SpiExpressionList { /** * Return a copy of the expression list. - *

- * Each of the expressions are expected to be immutable and safe to reference. - *

*/ public DefaultExpressionList copy(Query query) { - DefaultExpressionList copy = new DefaultExpressionList<>(query, expr, null); - copy.list.addAll(list); + DefaultExpressionList copy = new DefaultExpressionList<>(query, expr, null, new ArrayList<>(list.size())); + for (SpiExpression expr : list) { + copy.list.add(expr.copy()); + } return copy; } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/expression/ExistsQueryExpression.java b/ebean-core/src/main/java/io/ebeaninternal/server/expression/ExistsQueryExpression.java index 1e8121e52..4a8717e4c 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/expression/ExistsQueryExpression.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/expression/ExistsQueryExpression.java @@ -27,6 +27,11 @@ final class ExistsQueryExpression implements SpiExpression, UnsupportedDocStoreE this.subQuery = null; } + @Override + public SpiExpression copy() { + return subQuery == null ? this : new ExistsQueryExpression(subQuery.copy(), not); + } + @Override public void prefixProperty(String path) { // do nothing diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/expression/FilterExpressionList.java b/ebean-core/src/main/java/io/ebeaninternal/server/expression/FilterExpressionList.java index 45d308eec..3b8c5005b 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/expression/FilterExpressionList.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/expression/FilterExpressionList.java @@ -27,7 +27,7 @@ public final class FilterExpressionList extends DefaultExpressionList { } public FilterExpressionList(FilterExprPath pathPrefix, ExpressionFactory expr, Query rootQuery) { - super(null, expr, null); + super(null, expr, null, new ArrayList<>()); this.pathPrefix = pathPrefix; this.rootQuery = rootQuery; } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/expression/InQueryExpression.java b/ebean-core/src/main/java/io/ebeaninternal/server/expression/InQueryExpression.java index 20d6a3d16..f922edbfe 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/expression/InQueryExpression.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/expression/InQueryExpression.java @@ -31,6 +31,11 @@ final class InQueryExpression extends AbstractExpression implements UnsupportedD this.bindParams = bindParams; } + @Override + public SpiExpression copy() { + return subQuery == null ? this : new InQueryExpression(propName, subQuery.copy(), not); + } + @Override public void simplify() { // do nothing @@ -43,7 +48,6 @@ final class InQueryExpression extends AbstractExpression implements UnsupportedD @Override public void prepareExpression(BeanQueryRequest request) { - CQuery subQuery = compileSubQuery(request); this.bindParams = subQuery.predicates().whereExprBindValues(); this.sql = subQuery.generatedSql().replace('\n', ' '); @@ -71,7 +75,6 @@ final class InQueryExpression extends AbstractExpression implements UnsupportedD @Override public void addSql(SpiExpressionRequest request) { - request.append(" (").append(propName).append(")"); if (not) { request.append(" not"); @@ -83,7 +86,6 @@ final class InQueryExpression extends AbstractExpression implements UnsupportedD @Override public void addBindValues(SpiExpressionRequest request) { - for (Object bindParam : bindParams) { request.addBindValue(bindParam); } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java b/ebean-core/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java index ceafabbb9..5c5ff7f14 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java @@ -412,21 +412,22 @@ public class DefaultOrmQuery extends AbstractQuery implements SpiQuery { } private void createExtraJoinsToSupportManyWhereClause() { - manyWhereJoins = new ManyWhereJoins(); + final var manyWhere = new ManyWhereJoins(); if (whereExpressions != null) { - whereExpressions.containsMany(beanDescriptor, manyWhereJoins); + whereExpressions.containsMany(beanDescriptor, manyWhere); } if (havingExpressions != null) { - havingExpressions.containsMany(beanDescriptor, manyWhereJoins); + havingExpressions.containsMany(beanDescriptor, manyWhere); } if (orderBy != null) { for (Property orderProperty : orderBy.getProperties()) { ElPropertyDeploy elProp = beanDescriptor.elPropertyDeploy(orderProperty.getProperty()); if (elProp != null && elProp.containsFormulaWithJoin()) { - manyWhereJoins.addFormulaWithJoin(elProp.elPrefix(), elProp.name()); + manyWhere.addFormulaWithJoin(elProp.elPrefix(), elProp.name()); } } } + manyWhereJoins = manyWhere; } /** diff --git a/ebean-core/src/test/java/io/ebeaninternal/server/expression/DefaultExpressionListTest.java b/ebean-core/src/test/java/io/ebeaninternal/server/expression/DefaultExpressionListTest.java index 1fa245832..b89590936 100644 --- a/ebean-core/src/test/java/io/ebeaninternal/server/expression/DefaultExpressionListTest.java +++ b/ebean-core/src/test/java/io/ebeaninternal/server/expression/DefaultExpressionListTest.java @@ -1,16 +1,20 @@ package io.ebeaninternal.server.expression; import io.ebean.ExpressionList; +import io.ebean.Query; +import io.ebeaninternal.api.SpiQuery; import org.junit.jupiter.api.Test; +import java.util.ArrayList; + import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; public class DefaultExpressionListTest extends BaseExpressionTest { - DefaultExpressionList exp() { - - return new DefaultExpressionList<>(null, new DefaultExpressionFactory(true, true), null); + return new DefaultExpressionList<>(null, new DefaultExpressionFactory(true, true), null, new ArrayList<>()); } private DefaultExpressionList spi(ExpressionList list) { @@ -95,4 +99,34 @@ public class DefaultExpressionListTest extends BaseExpressionTest { .isSameByBind(spi(exp().eq("a", 10).eq("b", 20)))).isFalse(); } + @SuppressWarnings("unchecked") + @Test + void copy() { + DefaultExpressionList orig = exp(); + orig.eq("a", 10).in("b", 11); + DefaultExpressionList copy = (DefaultExpressionList)orig.copy(mock(Query.class)); + + assertThat(copy).isNotSameAs(orig); + assertThat(copy.list).hasSize(2); + assertThat(copy.list.get(0)).isSameAs(orig.list.get(0)); + assertThat(copy.list.get(1)).isSameAs(orig.list.get(1)); + } + + @SuppressWarnings("unchecked") + @Test + void copy_withSubQuery() { + DefaultExpressionList orig = exp(); + SpiQuery inSubQuery = mock(SpiQuery.class); + SpiQuery existsSubQuery = mock(SpiQuery.class); + orig.eq("a", 10).in("name", inSubQuery).exists(existsSubQuery); + + DefaultExpressionList copy = (DefaultExpressionList)orig.copy(mock(Query.class)); + + assertThat(copy.list).hasSize(3); + assertThat(copy.list.get(0)).isSameAs(orig.list.get(0)); + assertThat(copy.list.get(1)).isNotSameAs(orig.list.get(1)); + assertThat(copy.list.get(2)).isNotSameAs(orig.list.get(2)); + verify(inSubQuery).copy(); + verify(existsSubQuery).copy(); + } } diff --git a/ebean-core/src/test/java/io/ebeaninternal/server/expression/ExistsQueryExpressionTest.java b/ebean-core/src/test/java/io/ebeaninternal/server/expression/ExistsQueryExpressionTest.java index b869096a1..37433fbdf 100644 --- a/ebean-core/src/test/java/io/ebeaninternal/server/expression/ExistsQueryExpressionTest.java +++ b/ebean-core/src/test/java/io/ebeaninternal/server/expression/ExistsQueryExpressionTest.java @@ -1,10 +1,14 @@ package io.ebeaninternal.server.expression; +import io.ebeaninternal.api.SpiExpression; +import io.ebeaninternal.api.SpiQuery; import org.junit.jupiter.api.Test; import java.util.Arrays; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; public class ExistsQueryExpressionTest extends BaseExpressionTest { @@ -13,6 +17,22 @@ public class ExistsQueryExpressionTest extends BaseExpressionTest { return new ExistsQueryExpression(not, sql, Arrays.asList(bindValues)); } + @Test + void copy_subQuery_expectNewInstance() { + SpiQuery subQuery = mock(SpiQuery.class); + var orig = new ExistsQueryExpression(subQuery, false); + SpiExpression copy = orig.copy(); + assertThat(copy).isNotSameAs(orig); + verify(subQuery).copy(); + } + + @Test + void copy_sqlLiteral_expectSameInstance() { + var orig = exp(true, "sql", 10); + SpiExpression copy = orig.copy(); + assertThat(copy).isSameAs(orig); + } + @Test public void isSameByPlan_when_same() { diff --git a/ebean-core/src/test/java/io/ebeaninternal/server/expression/InQueryExpressionTest.java b/ebean-core/src/test/java/io/ebeaninternal/server/expression/InQueryExpressionTest.java index 1ab4853b5..b8d9fc451 100644 --- a/ebean-core/src/test/java/io/ebeaninternal/server/expression/InQueryExpressionTest.java +++ b/ebean-core/src/test/java/io/ebeaninternal/server/expression/InQueryExpressionTest.java @@ -1,18 +1,37 @@ package io.ebeaninternal.server.expression; +import io.ebeaninternal.api.SpiExpression; +import io.ebeaninternal.api.SpiQuery; import org.junit.jupiter.api.Test; import java.util.Arrays; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; public class InQueryExpressionTest extends BaseExpressionTest { - private InQueryExpression exp(String propertyName, boolean not, String sql, Object... bindValues) { return new InQueryExpression(propertyName, not, sql, Arrays.asList(bindValues)); } + @Test + void copy_subQuery_expectNewInstance() { + SpiQuery subQuery = mock(SpiQuery.class); + var orig = new InQueryExpression("name", subQuery, false); + SpiExpression copy = orig.copy(); + assertThat(copy).isNotSameAs(orig); + verify(subQuery).copy(); + } + + @Test + void copy_sqlLiteral_expectSameInstance() { + var orig = exp("name", true, "sql", 10); + SpiExpression copy = orig.copy(); + assertThat(copy).isSameAs(orig); + } + @Test public void isSameByPlan_when_same() { diff --git a/ebean-core/src/test/java/io/ebeaninternal/server/expression/JunctionExpressionTest.java b/ebean-core/src/test/java/io/ebeaninternal/server/expression/JunctionExpressionTest.java index b28d44c05..b1d4db672 100644 --- a/ebean-core/src/test/java/io/ebeaninternal/server/expression/JunctionExpressionTest.java +++ b/ebean-core/src/test/java/io/ebeaninternal/server/expression/JunctionExpressionTest.java @@ -5,16 +5,16 @@ import io.ebean.Expression; import io.ebean.Junction; import org.junit.jupiter.api.Test; +import java.util.ArrayList; + public class JunctionExpressionTest extends BaseExpressionTest { Expression eq(String propName, int value) { return Expr.eq(propName, value); } - DefaultExpressionList exp(Expression... expressions) { - - DefaultExpressionList list = new DefaultExpressionList<>(null, new DefaultExpressionFactory(true, false), null); + DefaultExpressionList list = new DefaultExpressionList<>(null, new DefaultExpressionFactory(true, false), null, new ArrayList<>()); for (Expression ex : expressions) { list.add(ex); } diff --git a/ebean-test/src/test/java/org/tests/query/TestQueryFindPagedList.java b/ebean-test/src/test/java/org/tests/query/TestQueryFindPagedList.java index 69092641e..629776577 100644 --- a/ebean-test/src/test/java/org/tests/query/TestQueryFindPagedList.java +++ b/ebean-test/src/test/java/org/tests/query/TestQueryFindPagedList.java @@ -70,11 +70,12 @@ public class TestQueryFindPagedList extends BaseTestCase { .setMaxRows(3) .findPagedList(); - Future rowCount = pagedList.getFutureCount(); + pagedList.loadCount(); List orders = pagedList.getList(); // these are each getting the total row count int totalRowCount = pagedList.getTotalCount(); + Future rowCount = pagedList.getFutureCount(); Integer totalRowCountWithTimeout = rowCount.get(30, TimeUnit.SECONDS); Integer totalRowCountViaFuture = rowCount.get();