From a1f95bf25ed60942f0b6dd65d6c02ef2f6b6074a Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Mon, 19 Jun 2017 12:05:56 +0200 Subject: [PATCH] Pr/id in as collection2 (#1034) * ENH: Changed idIn(List) to idIn(Collection) * ADD: sanity checks in IdBinder & special case if ID-list is empty --- src/main/java/io/ebean/ExpressionFactory.java | 5 +- src/main/java/io/ebean/ExpressionList.java | 4 +- .../server/deploy/id/IdBinderEmbedded.java | 7 ++- .../server/deploy/id/IdBinderSimple.java | 3 ++ .../expression/DefaultExpressionFactory.java | 7 ++- .../expression/DefaultExpressionList.java | 4 +- .../server/expression/DocQueryContext.java | 4 +- .../expression/FilterExpressionList.java | 4 +- .../server/expression/IdInExpression.java | 53 +++++++++++-------- .../server/expression/JunctionExpression.java | 2 +- 10 files changed, 55 insertions(+), 38 deletions(-) diff --git a/src/main/java/io/ebean/ExpressionFactory.java b/src/main/java/io/ebean/ExpressionFactory.java index 9ca504ce6..c0447f6ba 100644 --- a/src/main/java/io/ebean/ExpressionFactory.java +++ b/src/main/java/io/ebean/ExpressionFactory.java @@ -7,7 +7,6 @@ import io.ebean.search.TextQueryString; import io.ebean.search.TextSimple; import java.util.Collection; -import java.util.List; import java.util.Map; /** @@ -308,9 +307,9 @@ public interface ExpressionFactory { Expression idIn(Object... idValues); /** - * Id IN a list of Id values. + * Id IN a collection of Id values. */ - Expression idIn(List idList); + Expression idIn(Collection idCollection); /** * All Equal - Map containing property names and their values. diff --git a/src/main/java/io/ebean/ExpressionList.java b/src/main/java/io/ebean/ExpressionList.java index 24ab4bad9..af6ae8bcf 100644 --- a/src/main/java/io/ebean/ExpressionList.java +++ b/src/main/java/io/ebean/ExpressionList.java @@ -836,9 +836,9 @@ public interface ExpressionList { ExpressionList idIn(Object... idValues); /** - * Id IN a list of id values. + * Id IN a collection of id values. */ - ExpressionList idIn(List idValues); + ExpressionList idIn(Collection idValues); /** * Id Equal to - ID property is equal to the value. diff --git a/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderEmbedded.java b/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderEmbedded.java index e64a05be7..980ceef18 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderEmbedded.java +++ b/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderEmbedded.java @@ -172,6 +172,9 @@ public final class IdBinderEmbedded implements IdBinder { @Override public String getIdInValueExprDelete(int size) { + if (size <= 0) { + throw new IndexOutOfBoundsException("The size must be at least 1"); + } if (!idInExpandedForm) { return getIdInValueExpr(size); } @@ -199,7 +202,9 @@ public final class IdBinderEmbedded implements IdBinder { @Override public String getIdInValueExpr(int size) { - + if (size <= 0) { + throw new IndexOutOfBoundsException("The size must be at least 1"); + } StringBuilder sb = new StringBuilder(); if (!idInExpandedForm) { diff --git a/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderSimple.java b/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderSimple.java index 1072fbf23..c87eee837 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderSimple.java +++ b/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderSimple.java @@ -130,6 +130,9 @@ public final class IdBinderSimple implements IdBinder { @Override public String getIdInValueExpr(int size) { + if (size <= 0) { + throw new IndexOutOfBoundsException("The size must be at least 1"); + } StringBuilder sb = new StringBuilder(2 * size + 10); sb.append(" in"); sb.append(" (?"); diff --git a/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionFactory.java b/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionFactory.java index 269a65c61..ed473b201 100644 --- a/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionFactory.java +++ b/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionFactory.java @@ -18,7 +18,6 @@ import io.ebeaninternal.api.SpiQuery; import java.util.Arrays; import java.util.Collection; -import java.util.List; import java.util.Map; /** @@ -448,11 +447,11 @@ public class DefaultExpressionFactory implements SpiExpressionFactory { } /** - * Id IN a list of id values. + * Id IN a collection of id values. */ @Override - public Expression idIn(List idList) { - return new IdInExpression(idList); + public Expression idIn(Collection idCollection) { + return new IdInExpression(idCollection); } /** diff --git a/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionList.java b/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionList.java index 3d3d3432f..0d9881980 100644 --- a/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionList.java +++ b/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionList.java @@ -770,8 +770,8 @@ public class DefaultExpressionList implements SpiExpressionList { } @Override - public ExpressionList idIn(List idList) { - add(expr.idIn(idList)); + public ExpressionList idIn(Collection idCollection) { + add(expr.idIn(idCollection)); return this; } diff --git a/src/main/java/io/ebeaninternal/server/expression/DocQueryContext.java b/src/main/java/io/ebeaninternal/server/expression/DocQueryContext.java index abdf2e6a8..9f0195e85 100644 --- a/src/main/java/io/ebeaninternal/server/expression/DocQueryContext.java +++ b/src/main/java/io/ebeaninternal/server/expression/DocQueryContext.java @@ -10,7 +10,7 @@ import io.ebean.search.TextQueryString; import io.ebean.search.TextSimple; import java.io.IOException; -import java.util.List; +import java.util.Collection; import java.util.Map; /** @@ -66,7 +66,7 @@ public interface DocQueryContext { /** * Write an Id in expression. */ - void writeIds(List idList) throws IOException; + void writeIds(Collection idCollection) throws IOException; /** * Write an Id equals expression. diff --git a/src/main/java/io/ebeaninternal/server/expression/FilterExpressionList.java b/src/main/java/io/ebeaninternal/server/expression/FilterExpressionList.java index 801b58e9f..598fbf224 100644 --- a/src/main/java/io/ebeaninternal/server/expression/FilterExpressionList.java +++ b/src/main/java/io/ebeaninternal/server/expression/FilterExpressionList.java @@ -10,6 +10,8 @@ import io.ebean.Query; import io.ebeaninternal.api.SpiExpressionList; import javax.persistence.PersistenceException; + +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Set; @@ -95,7 +97,7 @@ public class FilterExpressionList extends DefaultExpressionList { } @Override - public ExpressionList idIn(List idValues) { + public ExpressionList idIn(Collection idValues) { throw new PersistenceException(notAllowedMessage); } diff --git a/src/main/java/io/ebeaninternal/server/expression/IdInExpression.java b/src/main/java/io/ebeaninternal/server/expression/IdInExpression.java index 712fea1dd..ca0c20fb6 100644 --- a/src/main/java/io/ebeaninternal/server/expression/IdInExpression.java +++ b/src/main/java/io/ebeaninternal/server/expression/IdInExpression.java @@ -9,17 +9,18 @@ import io.ebeaninternal.server.deploy.BeanDescriptor; import io.ebeaninternal.server.deploy.id.IdBinder; import java.io.IOException; -import java.util.List; +import java.util.Collection; +import java.util.Iterator; /** * Slightly redundant as Query.setId() ultimately also does the same job. */ public class IdInExpression extends NonPrepareExpression { - private final List idList; + private final Collection idCollection; - public IdInExpression(List idList) { - this.idList = idList; + public IdInExpression(Collection idCollection) { + this.idCollection = idCollection; } @Override @@ -33,7 +34,7 @@ public class IdInExpression extends NonPrepareExpression { @Override public void writeDocQuery(DocQueryContext context) throws IOException { - context.writeIds(idList); + context.writeIds(idCollection); } @Override @@ -50,8 +51,8 @@ public class IdInExpression extends NonPrepareExpression { BeanDescriptor descriptor = r.getBeanDescriptor(); IdBinder idBinder = descriptor.getIdBinder(); - for (Object anIdList : idList) { - idBinder.addIdInBindValue(request, anIdList); + for (Object id : idCollection) { + idBinder.addIdInBindValue(request, id); } } @@ -63,10 +64,13 @@ public class IdInExpression extends NonPrepareExpression { DefaultExpressionRequest r = (DefaultExpressionRequest) request; BeanDescriptor descriptor = r.getBeanDescriptor(); IdBinder idBinder = descriptor.getIdBinder(); - - request.append(descriptor.getIdBinder().getBindIdInSql(null)); - String inClause = idBinder.getIdInValueExpr(idList.size()); - request.append(inClause); + if (idCollection.size() == 0) { + request.append("1=0"); // append false for this stage + } else { + request.append(descriptor.getIdBinder().getBindIdInSql(null)); + String inClause = idBinder.getIdInValueExpr(idCollection.size()); + request.append(inClause); + } } @Override @@ -75,10 +79,13 @@ public class IdInExpression extends NonPrepareExpression { DefaultExpressionRequest r = (DefaultExpressionRequest) request; BeanDescriptor descriptor = r.getBeanDescriptor(); IdBinder idBinder = descriptor.getIdBinder(); - - request.append(descriptor.getIdBinderInLHSSql()); - String inClause = idBinder.getIdInValueExpr(idList.size()); - request.append(inClause); + if (idCollection.size() == 0) { + request.append("1=0"); // append false for this stage + } else { + request.append(descriptor.getIdBinderInLHSSql()); + String inClause = idBinder.getIdInValueExpr(idCollection.size()); + request.append(inClause); + } } /** @@ -86,13 +93,13 @@ public class IdInExpression extends NonPrepareExpression { */ @Override public void queryPlanHash(HashQueryPlanBuilder builder) { - builder.add(IdInExpression.class).add(idList.size()); - builder.bind(idList.size()); + builder.add(IdInExpression.class).add(idCollection.size()); + builder.bind(idCollection.size()); } @Override public int queryBindHash() { - return idList.hashCode(); + return idCollection.hashCode(); } @Override @@ -102,17 +109,19 @@ public class IdInExpression extends NonPrepareExpression { } IdInExpression that = (IdInExpression) other; - return this.idList.size() == that.idList.size(); + return this.idCollection.size() == that.idCollection.size(); } @Override public boolean isSameByBind(SpiExpression other) { IdInExpression that = (IdInExpression) other; - if (this.idList.size() != that.idList.size()) { + if (this.idCollection.size() != that.idCollection.size()) { return false; } - for (int i = 0; i < idList.size(); i++) { - if (!idList.get(i).equals(that.idList.get(i))) { + Iterator it = that.idCollection.iterator(); + for (Object id1 : idCollection) { + Object id2 = it.next(); + if (!id1.equals(id2)) { return false; } } diff --git a/src/main/java/io/ebeaninternal/server/expression/JunctionExpression.java b/src/main/java/io/ebeaninternal/server/expression/JunctionExpression.java index c4adbaf9c..6fa585c0d 100644 --- a/src/main/java/io/ebeaninternal/server/expression/JunctionExpression.java +++ b/src/main/java/io/ebeaninternal/server/expression/JunctionExpression.java @@ -568,7 +568,7 @@ class JunctionExpression implements SpiJunction, SpiExpression, Expression } @Override - public ExpressionList idIn(List idValues) { + public ExpressionList idIn(Collection idValues) { return exprList.idIn(idValues); }