Merge pull request #2408 from FOCONIS/pr/bugfix/reuse_update_collection_parameters

Fix reuse of io.ebean.Update when using setParameter with Collection

ebean-core/src/main/java/io/ebeaninternal/api/BindParams.java

@@ -285,12 +285,17 @@ public final class BindParams implements Serializable {
    */
   public boolean isSameBindHash() {
     if (bindHash == null) {
-      bindHash = calcQueryPlanHash();
       return false;
     }
-    String oldPlan = bindHash;
+    String newHash = calcQueryPlanHash();
+    return bindHash.equals(newHash);
+  }
+
+  /**
+   * Updates the hash.
+   */
+  public void updateHash() {
     bindHash = calcQueryPlanHash();
-    return bindHash.equals(oldPlan);
   }
 
   /**

ebean-core/src/main/java/io/ebeaninternal/server/persist/Binder.java

@@ -118,7 +118,11 @@ public final class Binder {
               bindLog.append(value);
             }
           }
-          if (value == null) {
+          if (value instanceof Collection) {
+            for (Object entry: (Collection<?>) value) {
+              bindObject(dataBind, entry);
+            }
+          } else if (value == null) {
             // this doesn't work for query predicates
             bindObject(dataBind, null, param.getType());
           } else {

ebean-core/src/main/java/io/ebeaninternal/server/util/BindParamsParser.java

@@ -79,6 +79,7 @@ public final class BindParamsParser {
       preparedSql = sql;
     }
     params.setPreparedSql(preparedSql);
+    params.updateHash();
     return preparedSql;
   }
 
@@ -151,18 +152,15 @@ public final class BindParamsParser {
         Object inValue = param.getInValue();
         if (inValue instanceof Collection<?>) {
           // Chop up Collection parameter into a number
-          // of individual parameters and add each one individually
+          // of individual parameters
           Collection<?> collection = (Collection<?>) inValue;
-          int c = 0;
-          for (Object elVal : collection) {
-            if (++c > 1) {
+          for (int c = 0; c < collection.size(); c++) {
+            if (c > 0) {
               orderedList.appendSql(",");
             }
             orderedList.appendSql("?");
-            BindParams.Param elParam = new BindParams.Param();
-            elParam.setInValue(elVal);
-            orderedList.add(elParam);
           }
+          orderedList.add(param);
 
         } else {
           // its a normal scalar value parameter...

ebean-test/src/test/java/io/ebeaninternal/api/BindParamsTest.java

@@ -19,17 +19,19 @@ public class BindParamsTest {
     BindParams.Param param = bindParams.getParameter("ids");
     assertEquals(3, param.queryBindCount());
     assertFalse(bindParams.isSameBindHash());
+    bindParams.updateHash();
 
     List<String> ids2 = Arrays.asList("1", "2", "3", "4");
     bindParams.setParameter("ids", ids2);
     assertEquals(4, param.queryBindCount());
     assertFalse(bindParams.isSameBindHash());
+    bindParams.updateHash();
 
     List<String> ids3 = Arrays.asList("2", "99", "44");
     bindParams.setParameter("ids", ids3);
     assertEquals(3, param.queryBindCount());
     assertFalse(bindParams.isSameBindHash());
-
+    bindParams.updateHash();
 
     List<String> ids4 = Arrays.asList("4545", "3499", "3444");
     bindParams.setParameter("ids", ids4);

ebean-test/src/test/java/org/tests/basic/TestUpdate.java

@@ -0,0 +1,71 @@
+package org.tests.basic;
+
+import io.ebean.BaseTestCase;
+import io.ebean.DB;
+import io.ebean.Update;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.tests.model.basic.Customer;
+
+import java.util.Arrays;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Testcase identified a bug when collections are used as bind parameters
+ *
+ * @author Roland Praml, FOCONIS AG
+ */
+public class TestUpdate extends BaseTestCase {
+
+  @BeforeEach
+  public void createCustomers() {
+    for (int i = 1; i <= 3; i++) {
+      Customer cust = new Customer();
+      cust.setName("testUpdate" + i);
+      DB.save(cust);
+    }
+  }
+
+  @AfterEach
+  public void deleteCustomers() {
+    DB.createUpdate(Customer.class, "delete from customer where name like 'testUpdate%'").execute();
+  }
+
+  @Test
+  public void testNormal() {
+
+    for (int i = 1; i <= 3; i++) {
+      Update<Customer> update = DB.createUpdate(Customer.class,
+        "update customer set smallnote = :smallnote where name in (:name)");
+      update.setParameter("name", Arrays.asList("testUpdate" + i)).setParameter("smallnote", "Note #" + i).execute();
+    }
+    Customer cust = DB.find(Customer.class).where().eq("name", "testUpdate3").findOne();
+    assertThat(cust.getSmallnote()).isEqualTo("Note #3");
+  }
+
+  @Test
+  public void testReuse() {
+
+    Update<Customer> update = DB.createUpdate(Customer.class,
+      "update customer set smallnote = :smallnote where name in (:name)");
+    for (int i = 1; i <= 3; i++) {
+      update.setParameter("name", Arrays.asList("testUpdate" + i)).setParameter("smallnote", "Note #" + i).execute();
+    }
+    Customer cust = DB.find(Customer.class).where().eq("name", "testUpdate3").findOne();
+    assertThat(cust.getSmallnote()).isEqualTo("Note #3");
+  }
+
+  @Test
+  public void testReuseNoArray() {
+
+    Update<Customer> update = DB.createUpdate(Customer.class,
+      "update customer set smallnote = :smallnote where name = :name");
+    for (int i = 1; i <= 3; i++) {
+      update.setParameter("name", "testUpdate" + i).setParameter("smallnote", "Note #" + i).execute();
+    }
+    Customer cust = DB.find(Customer.class).where().eq("name", "testUpdate3").findOne();
+    assertThat(cust.getSmallnote()).isEqualTo("Note #3");
+  }
+}