From 47b0a917a24ce0b27e83193670edc70823ec5e73 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Wed, 4 Oct 2017 23:55:48 +0200 Subject: [PATCH 01/10] No effective code change - fix imports and override annotations (#1155) --- src/main/java/io/ebean/RawSql.java | 11 ---------- .../io/ebeaninternal/json/DJsonService.java | 20 +++++++++++++++++++ .../server/deploy/BeanPropertyAssocMany.java | 1 + .../persist/dmlbind/FactoryProperty.java | 1 - .../server/text/json/WriteJson.java | 10 ++++++++++ .../update/TestUpdateAllLoadedProperties.java | 1 - 6 files changed, 31 insertions(+), 13 deletions(-) diff --git a/src/main/java/io/ebean/RawSql.java b/src/main/java/io/ebean/RawSql.java index 0c52bb83e..857ce7328 100644 --- a/src/main/java/io/ebean/RawSql.java +++ b/src/main/java/io/ebean/RawSql.java @@ -1,16 +1,5 @@ package io.ebean; -import io.ebean.util.CamelCaseHelper; - -import java.io.Serializable; -import java.sql.ResultSet; -import java.util.Collections; -import java.util.HashMap; -import java.util.Iterator; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; - /** * Used to build object graphs based on a raw SQL statement (rather than * generated by Ebean). diff --git a/src/main/java/io/ebeaninternal/json/DJsonService.java b/src/main/java/io/ebeaninternal/json/DJsonService.java index 8310041b0..0b4dc7412 100644 --- a/src/main/java/io/ebeaninternal/json/DJsonService.java +++ b/src/main/java/io/ebeaninternal/json/DJsonService.java @@ -22,6 +22,7 @@ public class DJsonService implements SpiJsonService { /** * Write the nested Map/List as json. */ + @Override public String write(Object object) throws IOException { return EJsonWriter.write(object); } @@ -29,6 +30,7 @@ public class DJsonService implements SpiJsonService { /** * Write the nested Map/List as json to the writer. */ + @Override public void write(Object object, Writer writer) throws IOException { EJsonWriter.write(object, writer); } @@ -36,6 +38,7 @@ public class DJsonService implements SpiJsonService { /** * Write the nested Map/List as json to the jsonGenerator. */ + @Override public void write(Object object, JsonGenerator jsonGenerator) throws IOException { EJsonWriter.write(object, jsonGenerator); } @@ -43,6 +46,7 @@ public class DJsonService implements SpiJsonService { /** * Write the collection as json array to the jsonGenerator. */ + @Override public void writeCollection(Collection collection, JsonGenerator jsonGenerator) throws IOException { EJsonWriter.writeCollection(collection, jsonGenerator); } @@ -51,6 +55,7 @@ public class DJsonService implements SpiJsonService { * Parse the json and return as a Map additionally specifying if the returned map should * be modify aware meaning that it can detect when it has been modified. */ + @Override public Map parseObject(String json, boolean modifyAware) throws IOException { return EJsonReader.parseObject(json, modifyAware); } @@ -58,6 +63,7 @@ public class DJsonService implements SpiJsonService { /** * Parse the json and return as a Map. */ + @Override public Map parseObject(String json) throws IOException { return EJsonReader.parseObject(json); } @@ -65,6 +71,7 @@ public class DJsonService implements SpiJsonService { /** * Parse the json and return as a Map taking a reader. */ + @Override public Map parseObject(Reader reader, boolean modifyAware) throws IOException { return EJsonReader.parseObject(reader, modifyAware); } @@ -72,6 +79,7 @@ public class DJsonService implements SpiJsonService { /** * Parse the json and return as a Map taking a reader. */ + @Override public Map parseObject(Reader reader) throws IOException { return EJsonReader.parseObject(reader); } @@ -79,6 +87,7 @@ public class DJsonService implements SpiJsonService { /** * Parse the json and return as a Map taking a JsonParser. */ + @Override public Map parseObject(JsonParser parser) throws IOException { return EJsonReader.parseObject(parser); } @@ -89,6 +98,7 @@ public class DJsonService implements SpiJsonService { * Used when the first token is checked to see if the value is null prior to calling this. *

*/ + @Override public Map parseObject(JsonParser parser, JsonToken token) throws IOException { return EJsonReader.parseObject(parser, token); } @@ -96,6 +106,7 @@ public class DJsonService implements SpiJsonService { /** * Parse the json and return as a modify aware List. */ + @Override public List parseList(String json, boolean modifyAware) throws IOException { return EJsonReader.parseList(json, modifyAware); } @@ -103,6 +114,7 @@ public class DJsonService implements SpiJsonService { /** * Parse the json and return as a List. */ + @Override public List parseList(String json) throws IOException { return EJsonReader.parseList(json); } @@ -110,6 +122,7 @@ public class DJsonService implements SpiJsonService { /** * Parse the json and return as a List taking a Reader. */ + @Override public List parseList(Reader reader) throws IOException { return EJsonReader.parseList(reader); } @@ -117,6 +130,7 @@ public class DJsonService implements SpiJsonService { /** * Parse the json and return as a List taking a JsonParser. */ + @Override public List parseList(JsonParser parser) throws IOException { return EJsonReader.parseList(parser, false); } @@ -124,6 +138,7 @@ public class DJsonService implements SpiJsonService { /** * Parse the json returning as a List taking into account the current token. */ + @Override @SuppressWarnings("unchecked") public List parseList(JsonParser parser, JsonToken currentToken) throws IOException { return (List) EJsonReader.parse(parser, currentToken, false); @@ -132,6 +147,7 @@ public class DJsonService implements SpiJsonService { /** * Parse the json and return as a List or Map. */ + @Override public Object parse(String json) throws IOException { return EJsonReader.parse(json); } @@ -139,6 +155,7 @@ public class DJsonService implements SpiJsonService { /** * Parse the json and return as a List or Map. */ + @Override public Object parse(Reader reader) throws IOException { return EJsonReader.parse(reader); } @@ -146,6 +163,7 @@ public class DJsonService implements SpiJsonService { /** * Parse the json and return as a List or Map. */ + @Override public Object parse(JsonParser parser) throws IOException { return EJsonReader.parse(parser); } @@ -153,6 +171,7 @@ public class DJsonService implements SpiJsonService { /** * Parse the json returning a Set that might be modify aware. */ + @Override public Set parseSet(String json, boolean modifyAware) throws IOException { List list = parseList(json, modifyAware); if (list == null) { @@ -169,6 +188,7 @@ public class DJsonService implements SpiJsonService { /** * Parse the json returning as a Set taking into account the current token. */ + @Override public Set parseSet(JsonParser parser, JsonToken currentToken) throws IOException { return new LinkedHashSet<>(parseList(parser, currentToken)); } diff --git a/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocMany.java b/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocMany.java index 7b3b54718..fd7024ac6 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocMany.java +++ b/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocMany.java @@ -965,6 +965,7 @@ public class BeanPropertyAssocMany extends BeanPropertyAssoc { /** * Skip JSON write value for ToMany property. */ + @Override public void jsonWriteValue(SpiJsonWriter writeJson, Object value) throws IOException { // do nothing, exclude ToMany properties } diff --git a/src/main/java/io/ebeaninternal/server/persist/dmlbind/FactoryProperty.java b/src/main/java/io/ebeaninternal/server/persist/dmlbind/FactoryProperty.java index dd1b8093d..a6f72b567 100644 --- a/src/main/java/io/ebeaninternal/server/persist/dmlbind/FactoryProperty.java +++ b/src/main/java/io/ebeaninternal/server/persist/dmlbind/FactoryProperty.java @@ -1,7 +1,6 @@ package io.ebeaninternal.server.persist.dmlbind; import io.ebeaninternal.server.deploy.BeanProperty; -import io.ebeaninternal.server.deploy.generatedproperty.GeneratedProperty; import io.ebeaninternal.server.persist.dml.DmlMode; /** diff --git a/src/main/java/io/ebeaninternal/server/text/json/WriteJson.java b/src/main/java/io/ebeaninternal/server/text/json/WriteJson.java index 7e65c7829..20c7d3f28 100644 --- a/src/main/java/io/ebeaninternal/server/text/json/WriteJson.java +++ b/src/main/java/io/ebeaninternal/server/text/json/WriteJson.java @@ -342,28 +342,34 @@ public class WriteJson implements SpiJsonWriter { } } + @Override public boolean isParentBean(Object bean) { return !parentBeans.isEmpty() && parentBeans.contains(bean); } + @Override public void pushParentBeanMany(EntityBean parentBean) { parentBeans.push(parentBean); } + @Override public void popParentBeanMany() { parentBeans.pop(); } + @Override public void beginAssocOne(String key, EntityBean bean) { parentBeans.push(bean); pathStack.pushPathKey(key); } + @Override public void endAssocOne() { parentBeans.pop(); pathStack.pop(); } + @Override public void beginAssocMany(String key) { try { pathStack.pushPathKey(key); @@ -374,6 +380,7 @@ public class WriteJson implements SpiJsonWriter { } } + @Override public void endAssocMany() { try { pathStack.pop(); @@ -407,6 +414,7 @@ public class WriteJson implements SpiJsonWriter { return new WriteBean(desc, explicitAllProps, currentIncludeProps, bean, visitor); } + @Override public void writeValueUsingObjectMapper(String name, Object value) { if (!isIncludeEmpty()) { @@ -534,6 +542,7 @@ public class WriteJson implements SpiJsonWriter { } } + @Override public Boolean includeMany(String key) { if (fetchPath != null) { String fullPath = pathStack.peekFullPath(key); @@ -542,6 +551,7 @@ public class WriteJson implements SpiJsonWriter { return null; } + @Override public void toJson(String name, Collection c) { try { diff --git a/src/test/java/org/tests/update/TestUpdateAllLoadedProperties.java b/src/test/java/org/tests/update/TestUpdateAllLoadedProperties.java index 64e1f0a91..b3d4812cb 100644 --- a/src/test/java/org/tests/update/TestUpdateAllLoadedProperties.java +++ b/src/test/java/org/tests/update/TestUpdateAllLoadedProperties.java @@ -13,7 +13,6 @@ import java.util.List; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; public class TestUpdateAllLoadedProperties extends BaseTestCase { From 64bf6d6a3592ef60d621aed2ce54491eef5c13c2 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Thu, 5 Oct 2017 00:04:12 +0200 Subject: [PATCH 02/10] commit transaction only if active (#1156) --- src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java b/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java index 71242c64b..939aed95f 100644 --- a/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java +++ b/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java @@ -288,7 +288,7 @@ public final class OrmQueryRequest extends BeanRequest implements BeanQueryRe */ @Override public void endTransIfRequired() { - if (createdTransaction) { + if (createdTransaction && transaction.isActive()) { transaction.commit(); } } From 1651d5e119a9612df98a29f8cfcef846b74305dc Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Thu, 5 Oct 2017 10:51:09 +0200 Subject: [PATCH 03/10] Misc platform fixes (#1157) * Oracle does not return row count - so we probably cannot detect Optimistic locking * DB2 wants a varchar null value (other platfroms are happy with this, too) * FIX Test for DB2 platform --- .../io/ebeaninternal/server/core/PersistRequestBean.java | 9 +++++++-- .../server/type/ScalarTypeJsonObjectMapper.java | 2 +- src/test/java/io/ebean/SqlRowBooleanTest.java | 2 ++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/ebeaninternal/server/core/PersistRequestBean.java b/src/main/java/io/ebeaninternal/server/core/PersistRequestBean.java index e9ee824cf..b36ecbf5a 100644 --- a/src/main/java/io/ebeaninternal/server/core/PersistRequestBean.java +++ b/src/main/java/io/ebeaninternal/server/core/PersistRequestBean.java @@ -31,6 +31,7 @@ import io.ebeanservice.docstore.api.DocStoreUpdates; import javax.persistence.OptimisticLockException; import javax.persistence.PersistenceException; import java.io.IOException; +import java.sql.Statement; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -784,8 +785,12 @@ public final class PersistRequestBean extends PersistRequest implements BeanP @Override public final void checkRowCount(int rowCount) { if (ConcurrencyMode.VERSION == concurrencyMode && rowCount != 1) { - String m = Message.msg("persist.conc2", String.valueOf(rowCount)); - throw new OptimisticLockException(m, null, bean); + // fix for oracle. + // see: https://stackoverflow.com/questions/19022175/executebatch-method-return-array-of-value-2-in-java + if (rowCount != Statement.SUCCESS_NO_INFO) { + String m = Message.msg("persist.conc2", String.valueOf(rowCount)); + throw new OptimisticLockException(m, null, bean); + } } switch (type) { case DELETE: diff --git a/src/main/java/io/ebeaninternal/server/type/ScalarTypeJsonObjectMapper.java b/src/main/java/io/ebeaninternal/server/type/ScalarTypeJsonObjectMapper.java index fa2e61837..eda46122a 100644 --- a/src/main/java/io/ebeaninternal/server/type/ScalarTypeJsonObjectMapper.java +++ b/src/main/java/io/ebeaninternal/server/type/ScalarTypeJsonObjectMapper.java @@ -184,7 +184,7 @@ public class ScalarTypeJsonObjectMapper { bind.setObject(PostgresHelper.asObject(pgType, rawJson)); } else { if (value == null) { - bind.setNull(Types.LONGNVARCHAR); // use longvarchar, otherwise SqlServer will fail with 'Invalid JDBC data type 5.001.' + bind.setNull(Types.VARCHAR); // use varchar, otherwise SqlServer/db2 will fail with 'Invalid JDBC data type 5.001.' } else { try { String json = objectMapper.writeValueAsString(value); diff --git a/src/test/java/io/ebean/SqlRowBooleanTest.java b/src/test/java/io/ebean/SqlRowBooleanTest.java index e1edbdb55..3496b02cd 100644 --- a/src/test/java/io/ebean/SqlRowBooleanTest.java +++ b/src/test/java/io/ebean/SqlRowBooleanTest.java @@ -14,6 +14,8 @@ public class SqlRowBooleanTest extends BaseTestCase { sqlQuery = Ebean.createSqlQuery("SELECT 1 AS ISNT_NULL"); } else if (isOracle()) { sqlQuery = Ebean.createSqlQuery("SELECT 1 AS ISNT_NULL from dual"); + } else if (isDb2()) { + sqlQuery = Ebean.createSqlQuery("SELECT 1 AS ISNT_NULL from SYSIBM.SYSDUMMY1"); } else { sqlQuery = Ebean.createSqlQuery("SELECT 1 IS NOT NULL AS ISNT_NULL"); } From bbe8abcc2ec5d31a0da7f8308aea80f9770da100 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Thu, 5 Oct 2017 22:42:05 +0200 Subject: [PATCH 04/10] Can handle one to many & many to one relations in docstore only entities (#1158) --- .../server/deploy/BeanDescriptorManager.java | 7 ++ .../server/deploy/BeanPropertyAssocOne.java | 2 +- .../tests/docstore/CustomerReportTest.java | 95 +++++++++++++++++++ .../tests/model/docstore/CustomerReport.java | 45 +++++++++ .../tests/model/docstore/ProductReport.java | 23 +++++ .../java/org/tests/model/docstore/Report.java | 33 +++++++ 6 files changed, 204 insertions(+), 1 deletion(-) create mode 100644 src/test/java/org/tests/docstore/CustomerReportTest.java create mode 100644 src/test/java/org/tests/model/docstore/CustomerReport.java create mode 100644 src/test/java/org/tests/model/docstore/ProductReport.java create mode 100644 src/test/java/org/tests/model/docstore/Report.java diff --git a/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorManager.java b/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorManager.java index 32faf53fa..9e5a5b528 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorManager.java +++ b/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorManager.java @@ -1038,6 +1038,13 @@ public class BeanDescriptorManager implements BeanDescriptorMap { } if (prop.getMappedBy() == null) { + // if we are doc store only we are done + // this allowes the use of @OneToMany in @DocStore - Entities + if (info.getDescriptor().isDocStoreOnly()) { + prop.setUnidirectional(); + return; + } + if (!findMappedBy(prop)) { makeUnidirectional(info, prop); return; diff --git a/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java b/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java index 1aebfd160..f4679cb19 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java +++ b/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java @@ -96,7 +96,7 @@ public class BeanPropertyAssocOne extends BeanPropertyAssoc { if (!isTransient) { //noinspection StatementWithEmptyBody - if (embedded) { + if (embedded || descriptor.isDocStoreOnly()) { // no imported or exported information } else if (!oneToOneExported) { importedId = createImportedId(this, targetDescriptor, tableJoin); diff --git a/src/test/java/org/tests/docstore/CustomerReportTest.java b/src/test/java/org/tests/docstore/CustomerReportTest.java new file mode 100644 index 000000000..3668d833f --- /dev/null +++ b/src/test/java/org/tests/docstore/CustomerReportTest.java @@ -0,0 +1,95 @@ +package org.tests.docstore; + + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Arrays; + +import org.junit.Test; +import org.tests.model.basic.Customer; +import org.tests.model.basic.Product; +import org.tests.model.basic.ResetBasicData; +import org.tests.model.docstore.CustomerReport; +import org.tests.model.docstore.ProductReport; + +import io.ebean.BaseTestCase; +import io.ebean.text.json.JsonReadOptions; + +public class CustomerReportTest extends BaseTestCase { + + + + @Test + public void testToJson() throws Exception { + ResetBasicData.reset(); + + + + String json = server().json().toJson(getCustomerReport()); + + + assertThat(json).isEqualTo("{\"dtype\":\"CR\",\"friends\":[{\"id\":2},{\"id\":3}],\"customer\":{\"id\":1}}"); + } + + + + @Test + public void testFromJson() throws Exception { + ResetBasicData.reset(); + String json = "{\"dtype\":\"CR\",\"friends\":[{\"id\":2},{\"id\":3}],\"customer\":{\"id\":1}}"; + + JsonReadOptions opts = new JsonReadOptions(); + opts.setEnableLazyLoading(true); + CustomerReport report = server().json().toBean(CustomerReport.class, json, opts); + + assertThat(report.getCustomer().getName()).isEqualTo("Rob"); + + assertThat(report.getFriends().get(0).getName()).isEqualTo("Cust NoAddress"); + assertThat(report.getFriends().get(1).getName()).isEqualTo("Fiona"); + } + + + @Test + public void testEmbeddedDocs() throws Exception { + ResetBasicData.reset(); + + CustomerReport report = getCustomerReport(); + report.getEmbeddedReports().add(getProductReport()); + + String json = server().json().toJson(report); + + assertThat(json).isEqualTo("{\"dtype\":\"CR\"," + + "\"embeddedReports\":[{\"dtype\":\"PR\",\"title\":\"This is a good product\",\"product\":{\"id\":1}}]," + + "\"friends\":[{\"id\":2},{\"id\":3}]," + + "\"customer\":{\"id\":1}}"); + + JsonReadOptions opts = new JsonReadOptions(); + opts.setEnableLazyLoading(true); + report = server().json().toBean(CustomerReport.class, json, opts); + ProductReport ar = (ProductReport) report.getEmbeddedReports().get(0); + assertThat(ar.getTitle()).isEqualTo("This is a good product"); + assertThat(ar.getProduct().getName()).isEqualTo("Chair"); + } + + + private CustomerReport getCustomerReport() { + Customer customer = server().getReference(Customer.class, 1); + Customer friend1 = server().getReference(Customer.class, 2); + Customer friend2 = server().getReference(Customer.class, 3); + + CustomerReport report = new CustomerReport(); + + report.setCustomer(customer); + report.setFriends(Arrays.asList(friend1, friend2)); + return report; + } + + private ProductReport getProductReport() { + Product product = server().getReference(Product.class, 1); + + ProductReport report = new ProductReport(); + report.setTitle("This is a good product"); + report.setProduct(product); + return report; + } +} diff --git a/src/test/java/org/tests/model/docstore/CustomerReport.java b/src/test/java/org/tests/model/docstore/CustomerReport.java new file mode 100644 index 000000000..74ffb0e92 --- /dev/null +++ b/src/test/java/org/tests/model/docstore/CustomerReport.java @@ -0,0 +1,45 @@ +package org.tests.model.docstore; + +import java.util.List; + +import javax.persistence.DiscriminatorValue; +import javax.persistence.ManyToOne; +import javax.persistence.OneToMany; + +import org.tests.model.basic.Customer; + +import io.ebean.annotation.DocStore; + +/** + * Entity that will stored as JSON in database + * + * @author Roland Praml, FOCONIS AG + * + */ +@DocStore +@DiscriminatorValue("CR") +public class CustomerReport extends Report { + + @OneToMany + private List friends; + + @ManyToOne + private Customer customer; + + public void setCustomer(Customer customer) { + this.customer = customer; + } + + public Customer getCustomer() { + return customer; + } + + public void setFriends(List friends) { + this.friends = friends; + } + + public List getFriends() { + return friends; + } + +} diff --git a/src/test/java/org/tests/model/docstore/ProductReport.java b/src/test/java/org/tests/model/docstore/ProductReport.java new file mode 100644 index 000000000..605ccde9a --- /dev/null +++ b/src/test/java/org/tests/model/docstore/ProductReport.java @@ -0,0 +1,23 @@ +package org.tests.model.docstore; + +import javax.persistence.DiscriminatorValue; +import javax.persistence.ManyToOne; + +import org.tests.model.basic.Product; + +import io.ebean.annotation.DocStore; + +@DocStore +@DiscriminatorValue("PR") +public class ProductReport extends Report { + + @ManyToOne + private Product product; + + public Product getProduct() { + return product; + } + public void setProduct(Product product) { + this.product = product; + } +} diff --git a/src/test/java/org/tests/model/docstore/Report.java b/src/test/java/org/tests/model/docstore/Report.java new file mode 100644 index 000000000..cefcbce10 --- /dev/null +++ b/src/test/java/org/tests/model/docstore/Report.java @@ -0,0 +1,33 @@ +package org.tests.model.docstore; + +import java.util.List; + +import javax.persistence.Inheritance; +import javax.persistence.OneToMany; + +import io.ebean.annotation.DocStore; + +@DocStore +@Inheritance +public class Report { + private String title; + + @OneToMany + private List embeddedReports; + + public void setTitle(String title) { + this.title = title; + } + + public String getTitle() { + return title; + } + + public List getEmbeddedReports() { + return embeddedReports; + } + + public void setEmbeddedReports(List embeddedReports) { + this.embeddedReports = embeddedReports; + } +} From ed08567d063a138a8af29a0d5cf95861063096db Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Thu, 5 Oct 2017 22:42:42 +0200 Subject: [PATCH 05/10] FIX: bundle symbolic name in manifest (#1159) --- src/main/resources/META-INF/MANIFEST.MF | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/META-INF/MANIFEST.MF b/src/main/resources/META-INF/MANIFEST.MF index dbbdd58a5..69302cc7e 100644 --- a/src/main/resources/META-INF/MANIFEST.MF +++ b/src/main/resources/META-INF/MANIFEST.MF @@ -3,7 +3,7 @@ Premain-Class: io.ebean.enhance.agent.Transformer Automatic-Module-Name: io.ebean Bundle-ManifestVersion: 2 Bundle-Name: Ebean-ORM -Bundle-SymbolicName: com.avaje.ebean +Bundle-SymbolicName: io.ebean Bundle-Version: 4.2.0 Bundle-ClassPath: . Bundle-Vendor: avaje From b2438ecd71dbf7a4b59945728b539590a5d1ab53 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Thu, 5 Oct 2017 22:43:30 +0200 Subject: [PATCH 06/10] FIX: do not generate unique-constraint for PK as it is useless (and oracle does not accept this) (#1160) --- .../dbmigration/ddlgeneration/platform/BaseTableDdl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/BaseTableDdl.java b/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/BaseTableDdl.java index 38e9f81ea..32ff43b08 100644 --- a/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/BaseTableDdl.java +++ b/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/BaseTableDdl.java @@ -577,7 +577,8 @@ public class BaseTableDdl implements TableDdl { List columns = createTable.getColumn(); for (Column column : columns) { - if (hasValue(column.getUnique()) || hasValue(column.getUniqueOneToOne())) { + if (!Boolean.TRUE.equals(column.isPrimaryKey()) + && (hasValue(column.getUnique()) || hasValue(column.getUniqueOneToOne()))) { if (Boolean.TRUE.equals(column.isNotnull()) || inlineUniqueWhenNullable) { // normal mechanism for adding unique constraint inlineUniqueConstraintSingle(apply, column); From e4eca4bbe95851804ab21d74db4116e8caa7fc69 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Thu, 5 Oct 2017 22:44:06 +0200 Subject: [PATCH 07/10] FIX: Oto relations must match datatype (#1161) --- src/test/java/org/tests/model/onetoone/OtoBChild.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/tests/model/onetoone/OtoBChild.java b/src/test/java/org/tests/model/onetoone/OtoBChild.java index 69603cf33..a2772a5e8 100644 --- a/src/test/java/org/tests/model/onetoone/OtoBChild.java +++ b/src/test/java/org/tests/model/onetoone/OtoBChild.java @@ -11,7 +11,7 @@ public class OtoBChild { @Id @Column(name = "master_id") - Integer id; + Long id; String child; @@ -19,11 +19,11 @@ public class OtoBChild { @PrimaryKeyJoinColumn(name = "master_id", referencedColumnName = "id") OtoBMaster master; - public Integer getId() { + public Long getId() { return id; } - public void setId(Integer id) { + public void setId(Long id) { this.id = id; } From 1909ba15576d42d196d8058c84d2ddc538be89b9 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Thu, 5 Oct 2017 22:46:56 +0200 Subject: [PATCH 08/10] Fix like escaping for DB2, Oracle and MySql (#1162) * Extended test case to show problem * FIX: Proper escaping of backslash in like/contains queries * Had to replace "like ?" assertions in tests with "like " as mysql will do a "like binary ? escape'|'" now to be case sensitive --- .../ebean/config/dbplatform/DatabasePlatform.java | 2 +- .../ebean/config/dbplatform/db2/DB2Platform.java | 9 ++++++++- .../config/dbplatform/mysql/MySqlPlatform.java | 8 ++++++++ .../config/dbplatform/oracle/OraclePlatform.java | 8 ++++++++ src/test/java/io/ebean/EbeanServer_eqlTest.java | 2 +- .../expression/DefaultExampleExpressionTest.java | 4 ++-- .../server/grammer/EqlParserTest.java | 2 +- .../batchload/TestBatchLazyWithCacheHits.java | 2 +- .../tests/query/TestExprNestedDisjunction.java | 4 ++-- .../tests/query/TestQueryFetchManyTwoDeep.java | 2 +- .../query/aggregation/TestAggregationCount.java | 2 +- .../joins/TestQueryManyToOneWhereClauseJoin.java | 6 +++--- .../orderby/TestOrderByWithDistinctTake2.java | 4 ++-- .../org/tests/query/other/TestLikeEscaping.java | 15 +++++++++++++-- .../query/other/TestQuerySingleAttribute.java | 2 +- 15 files changed, 53 insertions(+), 19 deletions(-) diff --git a/src/main/java/io/ebean/config/dbplatform/DatabasePlatform.java b/src/main/java/io/ebean/config/dbplatform/DatabasePlatform.java index 73c8d8274..6ae16cf34 100644 --- a/src/main/java/io/ebean/config/dbplatform/DatabasePlatform.java +++ b/src/main/java/io/ebean/config/dbplatform/DatabasePlatform.java @@ -175,7 +175,7 @@ public class DatabasePlatform { protected SqlExceptionTranslator exceptionTranslator = new SqlCodeTranslator(); - protected char[] specialLikeCharacters = { '%', '_' }; + protected char[] specialLikeCharacters = { '%', '_', '\\' }; /** * Instantiates a new database platform. diff --git a/src/main/java/io/ebean/config/dbplatform/db2/DB2Platform.java b/src/main/java/io/ebean/config/dbplatform/db2/DB2Platform.java index 0c9558cc4..ccc9b9fc6 100644 --- a/src/main/java/io/ebean/config/dbplatform/db2/DB2Platform.java +++ b/src/main/java/io/ebean/config/dbplatform/db2/DB2Platform.java @@ -26,7 +26,10 @@ public class DB2Platform extends DatabasePlatform { this.dbIdentity.setSupportsGetGeneratedKeys(true); this.dbIdentity.setSupportsSequence(true); - + + this.likeClause = "like ? escape '|'"; + this.specialLikeCharacters = new char[] { '%', '_', '|' }; + this.exceptionTranslator = new SqlErrorCodes() .addAcquireLock("40001","57033") // key -911/-913 @@ -46,6 +49,10 @@ public class DB2Platform extends DatabasePlatform { persistBatchOnCascade = PersistBatch.NONE; } + @Override + protected void escapeLikeCharacter(char ch, StringBuilder sb) { + sb.append('|').append(ch); + } /** * Return a DB2 specific sequence IdGenerator that supports batch fetching * sequence values. diff --git a/src/main/java/io/ebean/config/dbplatform/mysql/MySqlPlatform.java b/src/main/java/io/ebean/config/dbplatform/mysql/MySqlPlatform.java index 93b82d389..ee37d0c37 100644 --- a/src/main/java/io/ebean/config/dbplatform/mysql/MySqlPlatform.java +++ b/src/main/java/io/ebean/config/dbplatform/mysql/MySqlPlatform.java @@ -48,6 +48,9 @@ public class MySqlPlatform extends DatabasePlatform { this.openQuote = "`"; this.closeQuote = "`"; + // use pipe for escaping as it depends if mysql runs in no_backslash_escapes or not. + this.likeClause = "like binary ? escape '|'"; + this.specialLikeCharacters = new char[] { '%', '_', '|' }; this.forwardOnlyHintOnFindIterate = true; this.booleanDbType = Types.BIT; @@ -76,4 +79,9 @@ public class MySqlPlatform extends DatabasePlatform { // NOWAIT and SKIP LOCKED currently not supported with MySQL return sql + " for update"; } + + @Override + protected void escapeLikeCharacter(char ch, StringBuilder sb) { + sb.append('|').append(ch); + } } diff --git a/src/main/java/io/ebean/config/dbplatform/oracle/OraclePlatform.java b/src/main/java/io/ebean/config/dbplatform/oracle/OraclePlatform.java index 57bebab7d..b8c3c6a4a 100644 --- a/src/main/java/io/ebean/config/dbplatform/oracle/OraclePlatform.java +++ b/src/main/java/io/ebean/config/dbplatform/oracle/OraclePlatform.java @@ -37,6 +37,9 @@ public class OraclePlatform extends DatabasePlatform { this.treatEmptyStringsAsNull = true; + this.likeClause = "like ? escape '|'"; + this.specialLikeCharacters = new char[] { '%', '_', '|' }; + this.openQuote = "\""; this.closeQuote = "\""; @@ -78,4 +81,9 @@ public class OraclePlatform extends DatabasePlatform { return sql + " for update"; } } + + @Override + protected void escapeLikeCharacter(char ch, StringBuilder sb) { + sb.append('|').append(ch); + } } diff --git a/src/test/java/io/ebean/EbeanServer_eqlTest.java b/src/test/java/io/ebean/EbeanServer_eqlTest.java index eaf7c4482..360b3e941 100644 --- a/src/test/java/io/ebean/EbeanServer_eqlTest.java +++ b/src/test/java/io/ebean/EbeanServer_eqlTest.java @@ -135,7 +135,7 @@ public class EbeanServer_eqlTest extends BaseTestCase { query.setParameter("name", "Ro"); query.findList(); - assertThat(query.getGeneratedSql()).contains("where t0.name like ? "); + assertThat(query.getGeneratedSql()).contains("where t0.name like "); } @Test(expected = PersistenceException.class) diff --git a/src/test/java/io/ebeaninternal/server/expression/DefaultExampleExpressionTest.java b/src/test/java/io/ebeaninternal/server/expression/DefaultExampleExpressionTest.java index 29736f21a..00ba20dc2 100644 --- a/src/test/java/io/ebeaninternal/server/expression/DefaultExampleExpressionTest.java +++ b/src/test/java/io/ebeaninternal/server/expression/DefaultExampleExpressionTest.java @@ -86,8 +86,8 @@ public class DefaultExampleExpressionTest extends BaseExpressionTest { query1.findList(); - assertThat(query1.getGeneratedSql()).contains("(t0.name like ? "); - assertThat(query1.getGeneratedSql()).contains(" and t1.city like ? "); + assertThat(query1.getGeneratedSql()).contains("(t0.name like "); + assertThat(query1.getGeneratedSql()).contains(" and t1.city like "); } diff --git a/src/test/java/io/ebeaninternal/server/grammer/EqlParserTest.java b/src/test/java/io/ebeaninternal/server/grammer/EqlParserTest.java index 395680f48..03f043a8c 100644 --- a/src/test/java/io/ebeaninternal/server/grammer/EqlParserTest.java +++ b/src/test/java/io/ebeaninternal/server/grammer/EqlParserTest.java @@ -123,7 +123,7 @@ public class EqlParserTest extends BaseTestCase { query.setParameter("name", "Rob"); query.findList(); - assertThat(query.getGeneratedSql()).contains("where t0.name like ?"); + assertThat(query.getGeneratedSql()).contains("where t0.name like "); } @Test diff --git a/src/test/java/org/tests/batchload/TestBatchLazyWithCacheHits.java b/src/test/java/org/tests/batchload/TestBatchLazyWithCacheHits.java index c92149168..6246e4212 100644 --- a/src/test/java/org/tests/batchload/TestBatchLazyWithCacheHits.java +++ b/src/test/java/org/tests/batchload/TestBatchLazyWithCacheHits.java @@ -78,7 +78,7 @@ public class TestBatchLazyWithCacheHits extends BaseTestCase { // batch lazy loading into cache assertThat(sql).hasSize(2); - assertThat(sql.get(0)).contains("from uuone t0 where t0.name like ?"); + assertThat(sql.get(0)).contains("from uuone t0 where t0.name like "); assertThat(sql.get(1)).contains("from uuone t0 where t0.id in (?,"); statistics = beanCache.getStatistics(true); diff --git a/src/test/java/org/tests/query/TestExprNestedDisjunction.java b/src/test/java/org/tests/query/TestExprNestedDisjunction.java index c6df4fd0d..02125bc64 100644 --- a/src/test/java/org/tests/query/TestExprNestedDisjunction.java +++ b/src/test/java/org/tests/query/TestExprNestedDisjunction.java @@ -27,7 +27,7 @@ public class TestExprNestedDisjunction extends BaseTestCase { q.findList(); String s = q.getGeneratedSql(); - assertThat(s).contains("(t0.name like ? "); + assertThat(s).contains("(t0.name like "); assertThat(s).contains(" and t0.anniversary = ? ) or (t0.status = ? and t0.id > ? )"); } @@ -51,7 +51,7 @@ public class TestExprNestedDisjunction extends BaseTestCase { q.findList(); String s = q.getGeneratedSql(); - assertThat(s).contains("(t0.name like ? "); + assertThat(s).contains("(t0.name like "); assertThat(s).contains(" and t0.anniversary = ? ) or (t0.status = ? and t0.id > ? )"); } diff --git a/src/test/java/org/tests/query/TestQueryFetchManyTwoDeep.java b/src/test/java/org/tests/query/TestQueryFetchManyTwoDeep.java index bf667c46e..de846ef20 100644 --- a/src/test/java/org/tests/query/TestQueryFetchManyTwoDeep.java +++ b/src/test/java/org/tests/query/TestQueryFetchManyTwoDeep.java @@ -154,7 +154,7 @@ public class TestQueryFetchManyTwoDeep extends BaseTestCase { Assert.assertTrue(generatedSql.contains("from contact t0 ")); Assert.assertTrue(generatedSql.contains("join o_customer t1 on t1.id = t0.customer_id")); - Assert.assertTrue(generatedSql.contains("where lower(t1.name) like ?")); + Assert.assertTrue(generatedSql.contains("where lower(t1.name) like ")); } diff --git a/src/test/java/org/tests/query/aggregation/TestAggregationCount.java b/src/test/java/org/tests/query/aggregation/TestAggregationCount.java index 8b837b105..12d05c37b 100644 --- a/src/test/java/org/tests/query/aggregation/TestAggregationCount.java +++ b/src/test/java/org/tests/query/aggregation/TestAggregationCount.java @@ -82,7 +82,7 @@ public class TestAggregationCount extends BaseTestCase { String sql = sqlOf(query2, 5); assertThat(sql).contains("select t0.id, t0.name, count(u1.id), sum(u1.units), sum(u1.units * u1.amount) from tevent_one t0"); assertThat(sql).contains("from tevent_one t0 join tevent_many u1 on u1.event_id = t0.id "); - assertThat(sql).contains("where u1.description like ? "); + assertThat(sql).contains("where u1.description like "); assertThat(sql).contains(" group by t0.id, t0.name having count(u1.id) >= ? order by t0.name"); // invoke lazy loading diff --git a/src/test/java/org/tests/query/joins/TestQueryManyToOneWhereClauseJoin.java b/src/test/java/org/tests/query/joins/TestQueryManyToOneWhereClauseJoin.java index 4516d2dbe..abb33a17a 100644 --- a/src/test/java/org/tests/query/joins/TestQueryManyToOneWhereClauseJoin.java +++ b/src/test/java/org/tests/query/joins/TestQueryManyToOneWhereClauseJoin.java @@ -26,7 +26,7 @@ public class TestQueryManyToOneWhereClauseJoin extends BaseTestCase { query.findList(); //select t0.id c0, t0.status c1, t0.order_date c2, t0.ship_date c3, t1.name c4, t0.cretime c5, t0.updtime c6, t0.kcustomer_id c7 - String expectedSql = "from o_order t0 join o_customer t1 on t1.id = t0.kcustomer_id where lower(t1.name) like ? "; + String expectedSql = "from o_order t0 join o_customer t1 on t1.id = t0.kcustomer_id where lower(t1.name) like "; Assert.assertTrue(query.getGeneratedSql().contains(expectedSql)); // select t0.id c0, t0.status c1, t0.order_date c2, t0.ship_date c3, t1.name c4, t0.cretime c5, t0.updtime c6, t0.kcustomer_id c7 @@ -50,7 +50,7 @@ public class TestQueryManyToOneWhereClauseJoin extends BaseTestCase { query.findList(); //select t0.id c0, t0.status c1, t0.order_date c2, t0.ship_date c3, t1.name c4, t0.cretime c5, t0.updtime c6, t0.kcustomer_id c7 - String expectedSql = "from o_order t0 join o_customer t1 on t1.id = t0.kcustomer_id where (lower(t1.name) like ? "; + String expectedSql = "from o_order t0 join o_customer t1 on t1.id = t0.kcustomer_id where (lower(t1.name) like "; Assert.assertTrue(query.getGeneratedSql().contains(expectedSql)); // select t0.id c0, t0.status c1, t0.order_date c2, t0.ship_date c3, t1.name c4, t0.cretime c5, t0.updtime c6, t0.kcustomer_id c7 @@ -80,7 +80,7 @@ public class TestQueryManyToOneWhereClauseJoin extends BaseTestCase { String generatedSql = query.getGeneratedSql(); Assert.assertTrue(generatedSql.contains("from o_order t0 join o_customer t1 on t1.id = t0.kcustomer_id")); Assert.assertTrue(generatedSql.contains("left join contact t2 on t2.customer_id = t1.id")); - Assert.assertTrue(generatedSql.contains("where lower(t1.name) like ?")); + Assert.assertTrue(generatedSql.contains("where lower(t1.name) like ")); // select t0.id c0, t0.status c1, t0.order_date c2, t0.ship_date c3, t1.name c4, t0.cretime c5, t0.updtime c6, // t1.id c7, t1.status c8, t1.name c9, t1.smallnote c10, t1.anniversary c11, t1.cretime c12, t1.updtime c13, t1.billing_address_id c14, t1.shipping_address_id c15, diff --git a/src/test/java/org/tests/query/orderby/TestOrderByWithDistinctTake2.java b/src/test/java/org/tests/query/orderby/TestOrderByWithDistinctTake2.java index c80be3698..0f0123d0d 100644 --- a/src/test/java/org/tests/query/orderby/TestOrderByWithDistinctTake2.java +++ b/src/test/java/org/tests/query/orderby/TestOrderByWithDistinctTake2.java @@ -46,7 +46,7 @@ public class TestOrderByWithDistinctTake2 extends BaseTestCase { } assertThat(generatedSql).contains("order by t0.name desc"); assertThat(generatedSql).contains("from o_customer t0 join contact u1 on u1.customer_id = t0.id"); - assertThat(generatedSql).contains("where lower(u1.first_name) like ?"); + assertThat(generatedSql).contains("where lower(u1.first_name) like "); } @Test @@ -68,7 +68,7 @@ public class TestOrderByWithDistinctTake2 extends BaseTestCase { } assertThat(generatedSql).contains("order by t0.name, t0.id desc"); assertThat(generatedSql).contains("from o_customer t0 join contact u1 on u1.customer_id = t0.id"); - assertThat(generatedSql).contains("where lower(u1.first_name) like ?"); + assertThat(generatedSql).contains("where lower(u1.first_name) like "); } } diff --git a/src/test/java/org/tests/query/other/TestLikeEscaping.java b/src/test/java/org/tests/query/other/TestLikeEscaping.java index a9fbc5fe9..341348cca 100644 --- a/src/test/java/org/tests/query/other/TestLikeEscaping.java +++ b/src/test/java/org/tests/query/other/TestLikeEscaping.java @@ -22,11 +22,19 @@ public class TestLikeEscaping extends BaseTestCase { Ebean.save(ResetBasicData.createCustomer("Paul %% Doublepercentage", "|Pipeway", "[other]", 1, null)); Ebean.save(ResetBasicData.createCustomer("_Udo Underscore", "|Pipeway", "[other]", 1, null)); + Ebean.save(ResetBasicData.createCustomer("Bodo \\ backslash", "\\BS", "[other]", 1, null)); assertThat(Ebean.find(Customer.class) .where().contains("name", "Paul %%").findCount() ).isEqualTo(1); + assertThat(Ebean.find(Customer.class) + .where().contains("name", "o \\ b").findCount() + ).isEqualTo(1); + + assertThat(Ebean.find(Customer.class) + .where().contains("name", "o \\\\ b").findCount() + ).isEqualTo(0); assertThat(Ebean.find(Customer.class) .where().startsWith("name", "_").findCount() @@ -48,10 +56,13 @@ public class TestLikeEscaping extends BaseTestCase { .where().startsWith("shippingAddress.line1", "|P").findCount() ).isEqualTo(2); - + assertThat(Ebean.find(Customer.class) + .where().startsWith("shippingAddress.line1", "\\B").findCount() + ).isEqualTo(1); + assertThat(Ebean.find(Customer.class) .where().endsWith("billingAddress.line1", "]").findCount() - ).isEqualTo(4); + ).isEqualTo(5); assertThat(Ebean.find(Customer.class) .where().endsWith("billingAddress.line1", "[none]").findCount() diff --git a/src/test/java/org/tests/query/other/TestQuerySingleAttribute.java b/src/test/java/org/tests/query/other/TestQuerySingleAttribute.java index 2bc51aa76..b02a53c2f 100644 --- a/src/test/java/org/tests/query/other/TestQuerySingleAttribute.java +++ b/src/test/java/org/tests/query/other/TestQuerySingleAttribute.java @@ -112,7 +112,7 @@ public class TestQuerySingleAttribute extends BaseTestCase { List names = query.findSingleAttributeList(); - assertThat(sqlOf(query)).contains("select distinct t0.name from o_customer t0 left join o_address t1 on t1.id = t0.billing_address_id where t0.status = ? and lower(t1.city) like ?"); + assertThat(sqlOf(query)).contains("select distinct t0.name from o_customer t0 left join o_address t1 on t1.id = t0.billing_address_id where t0.status = ? and lower(t1.city) like "); assertThat(names).isNotNull(); } From fb1c7667cc2a8143d411286921989e467fce4f90 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Mon, 9 Oct 2017 09:28:36 +0200 Subject: [PATCH 09/10] FIX: order by must be added in findEach also (#1163) --- .../server/query/CQueryEngine.java | 24 +++++++++++------ .../tests/batchload/TestSecondaryQueries.java | 27 +++++++++++++++++++ 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/main/java/io/ebeaninternal/server/query/CQueryEngine.java b/src/main/java/io/ebeaninternal/server/query/CQueryEngine.java index 9315964de..51f53eb55 100644 --- a/src/main/java/io/ebeaninternal/server/query/CQueryEngine.java +++ b/src/main/java/io/ebeaninternal/server/query/CQueryEngine.java @@ -191,6 +191,7 @@ public class CQueryEngine { */ public QueryIterator findIterate(OrmQueryRequest request) { + prepareForPaging(request); CQuery cquery = queryBuilder.buildQuery(request); request.setCancelableQuery(cquery); @@ -326,18 +327,25 @@ public class CQueryEngine { return historySupport.getSysPeriodLower(rootTableAlias); } + /** + * deemed to be a be a paging query - check that the order by contains the id + * property to ensure unique row ordering for predicable paging but only in + * case, this is not a distinct query + * + * @param request + */ + private void prepareForPaging(OrmQueryRequest request) { + SpiQuery query = request.getQuery(); + if (!query.isDistinct() && (query.getMaxRows() > 1 || query.getFirstRow() > 0)) { + request.getBeanDescriptor().appendOrderById(query); + } + } /** * Find a list/map/set of beans. */ BeanCollection findMany(OrmQueryRequest request) { - SpiQuery query = request.getQuery(); - if (!query.isDistinct() && (query.getMaxRows() > 1 || query.getFirstRow() > 0)) { - // deemed to be a be a paging query - check that the order by contains - // the id property to ensure unique row ordering for predicable paging - // but only in case, this is not a distinct query - request.getBeanDescriptor().appendOrderById(query); - } + prepareForPaging(request); CQuery cquery = queryBuilder.buildQuery(request); request.setCancelableQuery(cquery); @@ -376,7 +384,7 @@ public class CQueryEngine { if (cquery != null) { cquery.close(); } - if (query.isFutureFetch()) { + if (request.getQuery().isFutureFetch()) { // end the transaction for futureFindIds // as it had it's own transaction logger.debug("Future fetch completed!"); diff --git a/src/test/java/org/tests/batchload/TestSecondaryQueries.java b/src/test/java/org/tests/batchload/TestSecondaryQueries.java index 50486385b..e853b7119 100644 --- a/src/test/java/org/tests/batchload/TestSecondaryQueries.java +++ b/src/test/java/org/tests/batchload/TestSecondaryQueries.java @@ -12,6 +12,7 @@ import org.ebeantest.LoggedSqlCollector; import org.junit.Assert; import org.junit.Test; +import java.util.Iterator; import java.util.List; import static org.assertj.core.api.Assertions.assertThat; @@ -72,6 +73,32 @@ public class TestSecondaryQueries extends BaseTestCase { assertThat(trimSql(sql.get(0), 1)).contains("select t0.id, t0.name from o_customer t0 where t0.id in"); } + + @Test + public void fetchIterate() { + + ResetBasicData.reset(); + + LoggedSqlCollector.start(); + + Iterator orders = Ebean.find(Order.class) + .select("status") + .setMaxRows(10) + .setUseCache(false) + .findIterate(); + while (orders.hasNext()) { + orders.next(); // dummy read + } + List sql = LoggedSqlCollector.stop(); + + assertThat(sql).hasSize(1); + if (isSqlServer()) { + assertThat(trimSql(sql.get(0), 2)).contains("select top 10 t0.id, t0.status from o_order t0 order by t0.id"); + } else { + assertThat(trimSql(sql.get(0), 2)).contains("select t0.id, t0.status from o_order t0"); + } + + } @Test public void testSecQueryOneToMany() { From d0ca75c25bea163fe19a247d24c27db61dfdc10f Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Mon, 9 Oct 2017 09:46:02 +0200 Subject: [PATCH 10/10] FIX: connection leak in query-iterator (#1164) --- .../io/ebeaninternal/server/query/CQuery.java | 5 +++ .../server/query/CQueryIteratorSimple.java | 19 +++++++-- .../org/tests/query/TestQueryFindIterate.java | 40 ++++++++++++++++++- 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/ebeaninternal/server/query/CQuery.java b/src/main/java/io/ebeaninternal/server/query/CQuery.java index eca6e71ec..98ace2aa5 100644 --- a/src/main/java/io/ebeaninternal/server/query/CQuery.java +++ b/src/main/java/io/ebeaninternal/server/query/CQuery.java @@ -36,6 +36,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.NoSuchElementException; /** * An object that represents a SqlSelect statement. @@ -421,6 +422,7 @@ public class CQuery implements DbReadContext, CancelableQuery { if (!moveToNextRow()) { if (currentBean == null) { + nextBean = null; return false; } else { // the last bean @@ -508,6 +510,9 @@ public class CQuery implements DbReadContext, CancelableQuery { auditIterateNextBean(); } hasNextCache = false; + if (nextBean == null) { + throw new NoSuchElementException(); + } return nextBean; } diff --git a/src/main/java/io/ebeaninternal/server/query/CQueryIteratorSimple.java b/src/main/java/io/ebeaninternal/server/query/CQueryIteratorSimple.java index a113cf760..fb434a6de 100644 --- a/src/main/java/io/ebeaninternal/server/query/CQueryIteratorSimple.java +++ b/src/main/java/io/ebeaninternal/server/query/CQueryIteratorSimple.java @@ -14,6 +14,8 @@ class CQueryIteratorSimple implements QueryIterator { private final CQuery cquery; private final OrmQueryRequest request; + + private boolean closed; CQueryIteratorSimple(CQuery cquery, OrmQueryRequest request) { this.cquery = cquery; @@ -22,11 +24,17 @@ class CQueryIteratorSimple implements QueryIterator { @Override public boolean hasNext() { + boolean ret = false; try { request.flushPersistenceContextOnIterate(); - return cquery.hasNext(); + ret = cquery.hasNext(); + return ret; } catch (SQLException e) { throw cquery.createPersistenceException(e); + } finally { + if (!ret) { + close(); + } } } @@ -38,9 +46,12 @@ class CQueryIteratorSimple implements QueryIterator { @Override public void close() { - cquery.updateExecutionStatisticsIterator(); - cquery.close(); - request.endTransIfRequired(); + if (!closed) { + closed = true; + cquery.updateExecutionStatisticsIterator(); + cquery.close(); + request.endTransIfRequired(); + } } @Override diff --git a/src/test/java/org/tests/query/TestQueryFindIterate.java b/src/test/java/org/tests/query/TestQueryFindIterate.java index 6d17d5599..6600315e3 100644 --- a/src/test/java/org/tests/query/TestQueryFindIterate.java +++ b/src/test/java/org/tests/query/TestQueryFindIterate.java @@ -9,16 +9,17 @@ import org.tests.model.basic.Customer; import org.tests.model.basic.Order; import org.tests.model.basic.OrderShipment; import org.tests.model.basic.ResetBasicData; +import org.avaje.datasource.DataSourcePool; import org.ebeantest.LoggedSqlCollector; import org.junit.Test; import javax.persistence.PersistenceException; import java.util.List; +import java.util.NoSuchElementException; import java.util.concurrent.atomic.AtomicInteger; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; public class TestQueryFindIterate extends BaseTestCase { @@ -224,4 +225,39 @@ public class TestQueryFindIterate extends BaseTestCase { } }); } + + @Test + public void testCloseConnection() throws Exception { + ResetBasicData.reset(); + DataSourcePool dsPool = (DataSourcePool) server().getPluginApi().getDataSource(); + int startConns = dsPool.getStatus(false).getBusy(); + QueryIterator queryIterator = server().find(Customer.class) + .where() + .isNotNull("name") + .setMaxRows(3) + .order().asc("id") + .findIterate(); + + assertThat(dsPool.getStatus(false).getBusy()).isEqualTo(startConns + 1); + + assertTrue(queryIterator.hasNext()); + assertThat(queryIterator.next()).isNotNull(); + assertThat(dsPool.getStatus(false).getBusy()).isEqualTo(startConns + 1); + + assertTrue(queryIterator.hasNext()); + assertThat(queryIterator.next()).isNotNull(); + assertThat(dsPool.getStatus(false).getBusy()).isEqualTo(startConns + 1); + + assertTrue(queryIterator.hasNext()); + assertThat(queryIterator.next()).isNotNull(); + assertThat(dsPool.getStatus(false).getBusy()).isEqualTo(startConns + 1); + + assertFalse(queryIterator.hasNext()); + assertThat(dsPool.getStatus(false).getBusy()).isEqualTo(startConns); + try { + queryIterator.next(); + fail("noSuchElementException expected"); + } catch (NoSuchElementException e) {} + + } }