From db6f93e3833acf09c00777ad4af042f6b93a5ee6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20P=C3=B6hler?= Date: Wed, 6 Oct 2021 11:57:09 +0200 Subject: [PATCH] FIX: Reuse of io.ebean.Update with collection parameters did not work correctly --- .../java/io/ebeaninternal/api/BindParams.java | 11 ++- .../ebeaninternal/server/persist/Binder.java | 6 +- .../server/util/BindParamsParser.java | 12 ++-- .../io/ebeaninternal/api/BindParamsTest.java | 4 +- .../test/java/org/tests/basic/TestUpdate.java | 71 +++++++++++++++++++ 5 files changed, 92 insertions(+), 12 deletions(-) create mode 100644 ebean-test/src/test/java/org/tests/basic/TestUpdate.java diff --git a/ebean-core/src/main/java/io/ebeaninternal/api/BindParams.java b/ebean-core/src/main/java/io/ebeaninternal/api/BindParams.java index 7c5264892..151a0b899 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/api/BindParams.java +++ b/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); } /** diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/persist/Binder.java b/ebean-core/src/main/java/io/ebeaninternal/server/persist/Binder.java index 6783beb7d..ca81e4ac8 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/persist/Binder.java +++ b/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 { diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/util/BindParamsParser.java b/ebean-core/src/main/java/io/ebeaninternal/server/util/BindParamsParser.java index cd14831e5..743047f94 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/util/BindParamsParser.java +++ b/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... diff --git a/ebean-test/src/test/java/io/ebeaninternal/api/BindParamsTest.java b/ebean-test/src/test/java/io/ebeaninternal/api/BindParamsTest.java index eef160dd7..1a9a52669 100644 --- a/ebean-test/src/test/java/io/ebeaninternal/api/BindParamsTest.java +++ b/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 ids2 = Arrays.asList("1", "2", "3", "4"); bindParams.setParameter("ids", ids2); assertEquals(4, param.queryBindCount()); assertFalse(bindParams.isSameBindHash()); + bindParams.updateHash(); List ids3 = Arrays.asList("2", "99", "44"); bindParams.setParameter("ids", ids3); assertEquals(3, param.queryBindCount()); assertFalse(bindParams.isSameBindHash()); - + bindParams.updateHash(); List ids4 = Arrays.asList("4545", "3499", "3444"); bindParams.setParameter("ids", ids4); diff --git a/ebean-test/src/test/java/org/tests/basic/TestUpdate.java b/ebean-test/src/test/java/org/tests/basic/TestUpdate.java new file mode 100644 index 000000000..c890ac956 --- /dev/null +++ b/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 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 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 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"); + } +}