From a4698ea1b662d06e35adf3399164a33a3df58909 Mon Sep 17 00:00:00 2001
From: rob bygrave <robin.bygrave@gmail.com>
Date: Fri, 1 Mar 2019 18:24:00 +1300
Subject: [PATCH] #1643 - ENH: Add expression inOrEmpty() ... as a convenience
 for conditionally added a IN expression

---
 src/main/java/io/ebean/Expr.java              |  37 ++++++
 src/main/java/io/ebean/ExpressionFactory.java |  35 ++++++
 src/main/java/io/ebean/ExpressionList.java    |  35 ++++++
 .../expression/DefaultExpressionFactory.java  |  10 ++
 .../expression/DefaultExpressionList.java     |   6 +
 .../server/expression/InExpression.java       |  48 +++++--
 .../server/expression/JunctionExpression.java |   5 +
 .../server/expression/InExpressionTest.java   |  28 ++++-
 .../java/org/tests/query/TestWhereIn.java     | 118 +++++++++++++++++-
 9 files changed, 301 insertions(+), 21 deletions(-)

diff --git a/src/main/java/io/ebean/Expr.java b/src/main/java/io/ebean/Expr.java
index 3b6ec35b4..62ffee527 100644
--- a/src/main/java/io/ebean/Expr.java
+++ b/src/main/java/io/ebean/Expr.java
@@ -250,6 +250,43 @@ public class Expr {
     return Ebean.getExpressionFactory().in(propertyName, values);
   }
 
+  /**
+   * In where null or empty values means that no predicate is added to the query.
+   * <p>
+   * That is, only add the IN predicate if the values are not null or empty.
+   * <p>
+   * Without this we typically need to code an <code>if</code> block to only add
+   * the IN predicate if the collection is not empty like:
+   * </p>
+   *
+   * <h3>Without inOrEmpty()</h3>
+   * <pre>{@code
+   *
+   *   query.where() // add some predicates
+   *     .eq("status", Status.NEW);
+   *
+   *   if (ids != null && !ids.isEmpty()) {
+   *     query.where().in("customer.id", ids);
+   *   }
+   *
+   *   query.findList();
+   *
+   * }</pre>
+   *
+   * <h3>Using inOrEmpty()</h3>
+   * <pre>{@code
+   *
+   *   query.where()
+   *     .eq("status", Status.NEW)
+   *     .inOrEmpty("customer.id", ids)
+   *     .findList();
+   *
+   * }</pre>
+   */
+  public static Expression inOrEmpty(String propertyName, Collection<?> values) {
+    return Ebean.getExpressionFactory().inOrEmpty(propertyName, values);
+  }
+
   /**
    * Id Equal to - ID property is equal to the value.
    */
diff --git a/src/main/java/io/ebean/ExpressionFactory.java b/src/main/java/io/ebean/ExpressionFactory.java
index c37ef6e78..0ea9071e4 100644
--- a/src/main/java/io/ebean/ExpressionFactory.java
+++ b/src/main/java/io/ebean/ExpressionFactory.java
@@ -325,6 +325,41 @@ public interface ExpressionFactory {
    */
   Expression in(String propertyName, Collection<?> values);
 
+  /**
+   * In where null or empty values means that no predicate is added to the query.
+   * <p>
+   * That is, only add the IN predicate if the values are not null or empty.
+   * <p>
+   * Without this we typically need to code an <code>if</code> block to only add
+   * the IN predicate if the collection is not empty like:
+   * </p>
+   *
+   * <h3>Without inOrEmpty()</h3>
+   * <pre>{@code
+   *
+   *   query.where() // add some predicates
+   *     .eq("status", Status.NEW);
+   *
+   *   if (ids != null && !ids.isEmpty()) {
+   *     query.where().in("customer.id", ids);
+   *   }
+   *
+   *   query.findList();
+   *
+   * }</pre>
+   *
+   * <h3>Using inOrEmpty()</h3>
+   * <pre>{@code
+   *
+   *   query.where()
+   *     .eq("status", Status.NEW)
+   *     .inOrEmpty("customer.id", ids)
+   *     .findList();
+   *
+   * }</pre>
+   */
+  Expression inOrEmpty(String propertyName, Collection<?> values);
+
   /**
    * Not In - property has a value in the array of values.
    */
diff --git a/src/main/java/io/ebean/ExpressionList.java b/src/main/java/io/ebean/ExpressionList.java
index 94d6b3f1d..8f5cc16fb 100644
--- a/src/main/java/io/ebean/ExpressionList.java
+++ b/src/main/java/io/ebean/ExpressionList.java
@@ -1003,6 +1003,41 @@ public interface ExpressionList<T> {
    */
   ExpressionList<T> in(String propertyName, Collection<?> values);
 
+  /**
+   * In where null or empty values means that no predicate is added to the query.
+   * <p>
+   * That is, only add the IN predicate if the values are not null or empty.
+   * <p>
+   * Without this we typically need to code an <code>if</code> block to only add
+   * the IN predicate if the collection is not empty like:
+   * </p>
+   *
+   * <h3>Without inOrEmpty()</h3>
+   * <pre>{@code
+   *
+   *   query.where() // add some predicates
+   *     .eq("status", Status.NEW);
+   *
+   *   if (ids != null && !ids.isEmpty()) {
+   *     query.where().in("customer.id", ids);
+   *   }
+   *
+   *   query.findList();
+   *
+   * }</pre>
+   *
+   * <h3>Using inOrEmpty()</h3>
+   * <pre>{@code
+   *
+   *   query.where()
+   *     .eq("status", Status.NEW)
+   *     .inOrEmpty("customer.id", ids)
+   *     .findList();
+   *
+   * }</pre>
+   */
+  ExpressionList<T> inOrEmpty(String propertyName, Collection<?> values);
+
   /**
    * In - using a subQuery.
    * <p>
diff --git a/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionFactory.java b/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionFactory.java
index 968cc881e..e568c2cbd 100644
--- a/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionFactory.java
+++ b/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionFactory.java
@@ -455,6 +455,16 @@ public class DefaultExpressionFactory implements SpiExpressionFactory {
     return new InExpression(propertyName, values, false);
   }
 
+  /**
+   * In where null or empty values means that no predicate is added to the query.
+   * <p>
+   * That is, only add the IN predicate if the values are not null or empty.
+   */
+  @Override
+  public Expression inOrEmpty(String propertyName, Collection<?> values) {
+    return new InExpression(propertyName, values, false, true);
+  }
+
   /**
    * In - property has a value in the array of values.
    */
diff --git a/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionList.java b/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionList.java
index 1c57f28ab..1372a06d2 100644
--- a/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionList.java
+++ b/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionList.java
@@ -912,6 +912,12 @@ public class DefaultExpressionList<T> implements SpiExpressionList<T> {
     return this;
   }
 
+  @Override
+  public ExpressionList<T> inOrEmpty(String propertyName, Collection<?> values) {
+    add(expr.inOrEmpty(propertyName, values));
+    return this;
+  }
+
   @Override
   public ExpressionList<T> in(String propertyName, Object... values) {
     add(expr.in(propertyName, values));
diff --git a/src/main/java/io/ebeaninternal/server/expression/InExpression.java b/src/main/java/io/ebeaninternal/server/expression/InExpression.java
index ca0ba1730..656b32fe8 100644
--- a/src/main/java/io/ebeaninternal/server/expression/InExpression.java
+++ b/src/main/java/io/ebeaninternal/server/expression/InExpression.java
@@ -17,8 +17,16 @@ import java.util.List;
 
 class InExpression extends AbstractExpression {
 
+  private static final String SQL_TRUE = "1=1";
+  private static final String SQL_FALSE = "1=0";
+
   private final boolean not;
 
+  /**
+   * Set to true when adding "1=1" predicate (due to null or empty sourceValues).
+   */
+  private final boolean empty;
+
   private final Collection<?> sourceValues;
 
   private List<Object> bindValues;
@@ -26,18 +34,27 @@ class InExpression extends AbstractExpression {
   private boolean multiValueSupported;
 
   InExpression(String propertyName, Collection<?> sourceValues, boolean not) {
+    this(propertyName, sourceValues, not, false);
+  }
+
+  InExpression(String propertyName, Collection<?> sourceValues, boolean not, boolean orEmpty) {
     super(propertyName);
     this.sourceValues = sourceValues;
     this.not = not;
+    this.empty = orEmpty && (sourceValues == null || sourceValues.isEmpty());
   }
 
   InExpression(String propertyName, Object[] array, boolean not) {
     super(propertyName);
     this.sourceValues = Arrays.asList(array);
     this.not = not;
+    this.empty = false;
   }
 
   private List<Object> values() {
+    if (empty || sourceValues == null) {
+      return Collections.emptyList();
+    }
     List<Object> vals = new ArrayList<>(sourceValues.size());
     for (Object sourceValue : sourceValues) {
       assert sourceValue != null : "null is not allowed in in-queries";
@@ -48,8 +65,8 @@ class InExpression extends AbstractExpression {
 
   @Override
   public boolean naturalKey(NaturalKeyQueryData<?> data) {
-    // can't use naturalKey cache for NOT IN
-    if (not) {
+    // can't use naturalKey cache for NOT IN or when "empty"
+    if (not || empty) {
       return false;
     }
     List<Object> copy = data.matchIn(propName, bindValues);
@@ -70,11 +87,16 @@ class InExpression extends AbstractExpression {
 
   @Override
   public void writeDocQuery(DocQueryContext context) throws IOException {
-    context.writeIn(propName, values().toArray(), not);
+    if (!empty) {
+      context.writeIn(propName, values().toArray(), not);
+    }
   }
 
   @Override
   public void addBindValues(SpiExpressionRequest request) {
+    if (empty) {
+      return;
+    }
     for (Object value : bindValues) {
       if (value == null) {
         throw new NullPointerException("null values in 'in(...)' queries must be handled separately!");
@@ -108,10 +130,12 @@ class InExpression extends AbstractExpression {
 
   @Override
   public void addSql(SpiExpressionRequest request) {
-
+    if (empty) {
+      request.append(SQL_TRUE);
+      return;
+    }
     if (bindValues.isEmpty()) {
-      String expr = not ? "1=1" : "1=0";
-      request.append(expr);
+      request.append(not ? SQL_TRUE : SQL_FALSE);
       return;
     }
 
@@ -142,10 +166,14 @@ class InExpression extends AbstractExpression {
       builder.append("In[");
     }
     builder.append(propName);
-    builder.append(" ?");
-    if (!multiValueSupported) {
-      // query plan specific to the number of parameters in the IN clause
-      builder.append(bindValues.size());
+    if (empty) {
+      builder.append("empty");
+    } else {
+      builder.append(" ?");
+      if (!multiValueSupported) {
+        // query plan specific to the number of parameters in the IN clause
+        builder.append(bindValues.size());
+      }
     }
     builder.append("]");
   }
diff --git a/src/main/java/io/ebeaninternal/server/expression/JunctionExpression.java b/src/main/java/io/ebeaninternal/server/expression/JunctionExpression.java
index 59d00dfe7..ee920b048 100644
--- a/src/main/java/io/ebeaninternal/server/expression/JunctionExpression.java
+++ b/src/main/java/io/ebeaninternal/server/expression/JunctionExpression.java
@@ -672,6 +672,11 @@ class JunctionExpression<T> implements SpiJunction<T>, SpiExpression, Expression
     return exprList.in(propertyName, values);
   }
 
+  @Override
+  public ExpressionList<T> inOrEmpty(String propertyName, Collection<?> values) {
+    return exprList.inOrEmpty(propertyName, values);
+  }
+
   @Override
   public ExpressionList<T> in(String propertyName, Object... values) {
     return exprList.in(propertyName, values);
diff --git a/src/test/java/io/ebeaninternal/server/expression/InExpressionTest.java b/src/test/java/io/ebeaninternal/server/expression/InExpressionTest.java
index cd49f4a34..4b238562d 100644
--- a/src/test/java/io/ebeaninternal/server/expression/InExpressionTest.java
+++ b/src/test/java/io/ebeaninternal/server/expression/InExpressionTest.java
@@ -11,7 +11,25 @@ import static org.assertj.core.api.StrictAssertions.assertThat;
 public class InExpressionTest extends BaseExpressionTest {
 
   @Test
-  public void queryPlanHash_given_diffPropertyName_should_differentPlanHash() throws Exception {
+  public void queryPlanHash_given_diffEmpty_should_differentPlanHash() {
+
+    List<Integer> emptyValues = values();
+
+    InExpression ex1 = new InExpression("foo", emptyValues, false, true);
+    InExpression ex2 = new InExpression("foo", emptyValues, false);
+    InExpression ex3 = new InExpression("foo", emptyValues, false, true);
+    InExpression ex4 = new InExpression("foo", null, false, true);
+
+    ex1.prepareExpression(multi());
+    ex2.prepareExpression(multi());
+
+    different(ex1, ex2);
+    same(ex1, ex3); // same empty
+    same(ex1, ex4); // same null
+  }
+
+  @Test
+  public void queryPlanHash_given_diffPropertyName_should_differentPlanHash() {
 
     List<Integer> values = values(42, 92);
 
@@ -25,7 +43,7 @@ public class InExpressionTest extends BaseExpressionTest {
   }
 
   @Test
-  public void queryPlanHash_given_diffBindCount_should_differentPlanHash() throws Exception {
+  public void queryPlanHash_given_diffBindCount_should_differentPlanHash() {
 
     List<Integer> values1 = values(42, 92);
     List<Integer> values2 = values(42, 92, 82);
@@ -39,7 +57,7 @@ public class InExpressionTest extends BaseExpressionTest {
   }
 
   @Test
-  public void queryPlanHash_given_diffBindCount_withMultiSupport_samePlanHash() throws Exception {
+  public void queryPlanHash_given_diffBindCount_withMultiSupport_samePlanHash() {
 
     List<Integer> values1 = values(42, 92);
     List<Integer> values2 = values(42, 92, 82);
@@ -53,7 +71,7 @@ public class InExpressionTest extends BaseExpressionTest {
   }
 
   @Test
-  public void queryPlanHash_given_diffNotFlag_should_differentPlanHash() throws Exception {
+  public void queryPlanHash_given_diffNotFlag_should_differentPlanHash() {
 
     List<Integer> values = values(42, 92);
 
@@ -67,7 +85,7 @@ public class InExpressionTest extends BaseExpressionTest {
   }
 
   @Test
-  public void queryPlanHash_given_sameNotFlag_should_samePlanHash() throws Exception {
+  public void queryPlanHash_given_sameNotFlag_should_samePlanHash() {
 
     List<Integer> values = values(42, 92);
 
diff --git a/src/test/java/org/tests/query/TestWhereIn.java b/src/test/java/org/tests/query/TestWhereIn.java
index 4be3ecb91..c173bbc30 100644
--- a/src/test/java/org/tests/query/TestWhereIn.java
+++ b/src/test/java/org/tests/query/TestWhereIn.java
@@ -1,12 +1,19 @@
 package org.tests.query;
 
 import io.ebean.BaseTestCase;
-import io.ebean.Ebean;
+import io.ebean.DB;
 import io.ebean.Query;
 import org.junit.Test;
 import org.tests.model.basic.Country;
+import org.tests.model.basic.Order;
 import org.tests.model.basic.ResetBasicData;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
 
 public class TestWhereIn extends BaseTestCase {
 
@@ -15,25 +22,124 @@ public class TestWhereIn extends BaseTestCase {
 
     ResetBasicData.reset();
 
-    Query<Country> query = Ebean.find(Country.class)
+    Query<Country> query = DB.find(Country.class)
       .where().in("code", "NZ", "AU")
       .query();
 
     query.findList();
-    platformAssertIn(sqlOf(query), "");
+    platformAssertIn(sqlOf(query), "where t0.code");
   }
 
-
   @Test
   public void testNotInVarchar() {
 
     ResetBasicData.reset();
 
-    Query<Country> query = Ebean.find(Country.class)
+    Query<Country> query = DB.find(Country.class)
       .where().notIn("code", "NZ", "SA", "US")
       .query();
 
     query.findList();
-    platformAssertNotIn(sqlOf(query), "");
+    platformAssertNotIn(sqlOf(query), "where t0.code");
+  }
+
+  @Test
+  public void testInOrEmpty_expect_noJoinWhenEmpty() {
+
+    ResetBasicData.reset();
+
+    Query<Order> query = DB.find(Order.class)
+      .select("id")
+      .where().inOrEmpty("customer.billingAddress.id", new ArrayList<>()).query();
+
+    query.findList();
+    assertThat(sqlOf(query)).isEqualTo("select t0.id from o_order t0 where 1=1");
+  }
+
+  @Test
+  public void testInOrEmpty_expect_joinWhenNotEmpty() {
+
+    ResetBasicData.reset();
+
+    Query<Order> query = DB.find(Order.class)
+      .select("id")
+      .where().inOrEmpty("customer.billingAddress.id", Arrays.asList(1)).query();
+
+    query.findList();
+    assertThat(sqlOf(query)).contains("select t0.id from o_order t0 join o_customer t1 on t1.id = t0.kcustomer_id  where t1.billing_address_id ");
+  }
+
+
+  @Test
+  public void testInOrEmpty_when_null() {
+
+    ResetBasicData.reset();
+
+    Query<Country> query = DB.find(Country.class)
+      .where().inOrEmpty("code", null).query();
+
+    query.findList();
+    assertThat(sqlOf(query)).contains("where 1=1");
+  }
+
+
+  @Test
+  public void testInOrEmpty_when_empty() {
+
+    ResetBasicData.reset();
+
+    Query<Country> query = DB.find(Country.class)
+      .where().inOrEmpty("code", new ArrayList<>()).query();
+
+    query.findList();
+    assertThat(sqlOf(query)).contains("where 1=1");
+  }
+
+  @Test
+  public void testIn_when_empty() {
+
+    ResetBasicData.reset();
+
+    Query<Country> query = DB.find(Country.class)
+      .where().in("code", new ArrayList<>()).query();
+
+    query.findList();
+    assertThat(sqlOf(query)).contains("where 1=0");
+  }
+
+  @Test
+  public void testIn_when_null() {
+
+    ResetBasicData.reset();
+
+    Query<Country> query = DB.find(Country.class)
+      .where().in("code", (Collection)null).query();
+
+    query.findList();
+    assertThat(sqlOf(query)).contains("where 1=0");
+  }
+
+  @Test
+  public void testNotIn_when_empty() {
+
+    ResetBasicData.reset();
+
+    Query<Country> query = DB.find(Country.class)
+      .where().notIn("code", new ArrayList<>()).query();
+
+    query.findList();
+    assertThat(sqlOf(query)).contains("where 1=1");
+  }
+
+  @Test
+  public void testNotIn_when_null() {
+
+    ResetBasicData.reset();
+
+    Query<Country> query = DB.find(Country.class)
+      .where().notIn("code", (Collection)null).query();
+
+    query.findList();
+    assertThat(sqlOf(query)).contains("where 1=1");
   }
 }