From e4088c5fbe45ebfb61e89c51d62e73d4ed52adb2 Mon Sep 17 00:00:00 2001 From: rob bygrave Date: Wed, 5 Dec 2018 23:03:15 +1300 Subject: [PATCH] #1571 - NPE in cacheable bean with embeddedid on delete --- .../server/deploy/AssocOneHelp.java | 19 +++++++++ .../server/deploy/AssocOneHelpEmbedded.java | 42 +++++++++++++++---- .../server/deploy/BeanProperty.java | 18 ++++++++ .../server/deploy/BeanPropertyAssocOne.java | 22 ++++++---- .../DynamicPropertyAggregationFormula.java | 11 +++++ .../query/CQueryFetchSingleAttribute.java | 10 ++--- .../server/query/CQueryPlan.java | 6 +-- .../server/query/STreeProperty.java | 3 +- .../server/query/STreePropertyAssocOne.java | 4 +- .../server/query/SqlTreeNode.java | 6 +-- .../server/query/SqlTreeNodeBean.java | 12 +++--- .../server/query/SqlTreeNodeExtraJoin.java | 2 +- .../query/SqlTreeNodeManyWhereJoin.java | 2 +- .../server/type/ScalarDataReader.java | 10 ----- .../ebeaninternal/server/type/ScalarType.java | 6 +-- .../tests/compositekeys/TestCKeyDelete.java | 31 +++++++++----- 16 files changed, 142 insertions(+), 62 deletions(-) diff --git a/src/main/java/io/ebeaninternal/server/deploy/AssocOneHelp.java b/src/main/java/io/ebeaninternal/server/deploy/AssocOneHelp.java index adfb689a2..0fbf29da6 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/AssocOneHelp.java +++ b/src/main/java/io/ebeaninternal/server/deploy/AssocOneHelp.java @@ -3,6 +3,7 @@ package io.ebeaninternal.server.deploy; import io.ebean.bean.EntityBean; import io.ebean.bean.PersistenceContext; import io.ebeaninternal.server.query.SqlJoinType; +import io.ebeaninternal.server.type.DataReader; import java.sql.SQLException; @@ -28,6 +29,24 @@ abstract class AssocOneHelp { property.targetIdBinder.loadIgnore(ctx); } + /** + * Read and return the property. + */ + Object read(DataReader reader) throws SQLException { + return property.read(reader); + } + + /** + * Read and return the property setting value into the bean. + */ + Object readSet(DataReader reader, EntityBean bean) throws SQLException { + Object val = read(reader); + if (bean != null) { + property.setValue(bean, val); + } + return val; + } + /** * Read and return the bean. */ diff --git a/src/main/java/io/ebeaninternal/server/deploy/AssocOneHelpEmbedded.java b/src/main/java/io/ebeaninternal/server/deploy/AssocOneHelpEmbedded.java index b15650e53..a767b2e14 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/AssocOneHelpEmbedded.java +++ b/src/main/java/io/ebeaninternal/server/deploy/AssocOneHelpEmbedded.java @@ -1,6 +1,7 @@ package io.ebeaninternal.server.deploy; import io.ebean.bean.EntityBean; +import io.ebeaninternal.server.type.DataReader; import java.sql.SQLException; @@ -9,14 +10,41 @@ import java.sql.SQLException; */ final class AssocOneHelpEmbedded extends AssocOneHelp { - public AssocOneHelpEmbedded(BeanPropertyAssocOne property) { + AssocOneHelpEmbedded(BeanPropertyAssocOne property) { super(property); } @Override void loadIgnore(DbReadContext ctx) { - for (int i = 0; i < property.embeddedProps.length; i++) { - property.embeddedProps[i].loadIgnore(ctx); + for (BeanProperty property : property.embeddedProps) { + property.loadIgnore(ctx); + } + } + + @Override + Object readSet(DataReader reader, EntityBean bean) throws SQLException { + Object dbVal = read(reader); + if (bean != null) { + property.setValue(bean, dbVal); + } + return dbVal; + } + + @Override + Object read(DataReader reader) throws SQLException { + + EntityBean embeddedBean = property.targetDescriptor.createEntityBean(); + boolean notNull = false; + for (BeanProperty property : property.embeddedProps) { + Object value = property.readSet(reader, embeddedBean); + if (value != null) { + notNull = true; + } + } + if (notNull) { + return embeddedBean; + } else { + return null; } } @@ -39,8 +67,8 @@ final class AssocOneHelpEmbedded extends AssocOneHelp { EntityBean embeddedBean = property.targetDescriptor.createEntityBean(); boolean notNull = false; - for (int i = 0; i < property.embeddedProps.length; i++) { - Object value = property.embeddedProps[i].readSet(ctx, embeddedBean); + for (BeanProperty property : property.embeddedProps) { + Object value = property.readSet(ctx, embeddedBean); if (value != null) { notNull = true; } @@ -55,8 +83,8 @@ final class AssocOneHelpEmbedded extends AssocOneHelp { @Override void appendSelect(DbSqlContext ctx, boolean subQuery) { - for (int i = 0; i < property.embeddedProps.length; i++) { - property.embeddedProps[i].appendSelect(ctx, subQuery); + for (BeanProperty property : property.embeddedProps) { + property.appendSelect(ctx, subQuery); } } } diff --git a/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java b/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java index 6641c158f..6ae467d44 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java +++ b/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java @@ -28,6 +28,7 @@ import io.ebeaninternal.server.query.STreeProperty; import io.ebeaninternal.server.query.SqlBeanLoad; import io.ebeaninternal.server.query.SqlJoinType; import io.ebeaninternal.server.type.DataBind; +import io.ebeaninternal.server.type.DataReader; import io.ebeaninternal.server.type.LocalEncryptedType; import io.ebeaninternal.server.type.ScalarType; import io.ebeaninternal.server.type.ScalarTypeBoolean; @@ -640,6 +641,23 @@ public class BeanProperty implements ElPropertyValue, Property, STreeProperty { } } + @Override + public Object read(DataReader reader) throws SQLException { + return scalarType.read(reader); + } + + public Object readSet(DataReader reader, EntityBean bean) throws SQLException { + try { + Object value = scalarType.read(reader); + if (bean != null) { + setValue(bean, value); + } + return value; + } catch (Exception e) { + throw new PersistenceException("Error readSet on " + descriptor + "." + name, e); + } + } + public Object read(DbReadContext ctx) throws SQLException { return scalarType.read(ctx.getDataReader()); } diff --git a/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java b/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java index bd3455d90..9a5f7c1c2 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java +++ b/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java @@ -21,7 +21,8 @@ import io.ebeaninternal.server.el.ElPropertyValue; import io.ebeaninternal.server.query.STreePropertyAssocOne; import io.ebeaninternal.server.query.SqlBeanLoad; import io.ebeaninternal.server.query.SqlJoinType; -import io.ebeaninternal.server.type.ScalarType; +import io.ebeaninternal.server.type.DataReader; +import io.ebeaninternal.server.type.ScalarDataReader; import javax.persistence.PersistenceException; import java.io.IOException; @@ -434,8 +435,8 @@ public class BeanPropertyAssocOne extends BeanPropertyAssoc implements STr } @Override - public ScalarType getIdScalarType() { - return targetDescriptor.getIdProperty().getScalarType(); + public ScalarDataReader getIdReader() { + return targetDescriptor.getIdProperty(); } /** @@ -583,18 +584,23 @@ public class BeanPropertyAssocOne extends BeanPropertyAssoc implements STr } } + @Override + public Object readSet(DataReader reader, EntityBean bean) throws SQLException { + return localHelp.readSet(reader, bean); + } + + @Override + public Object read(DataReader reader) throws SQLException { + return localHelp.read(reader); + } + @Override public Object readSet(DbReadContext ctx, EntityBean bean) throws SQLException { return localHelp.readSet(ctx, bean); } - /** - * Read the data from the resultSet effectively ignoring it and returning null. - */ @Override public Object read(DbReadContext ctx) throws SQLException { - // just read the resultSet incrementing the column index - // pass in null for the bean so any data read is ignored return localHelp.read(ctx); } diff --git a/src/main/java/io/ebeaninternal/server/deploy/DynamicPropertyAggregationFormula.java b/src/main/java/io/ebeaninternal/server/deploy/DynamicPropertyAggregationFormula.java index d238e7ca2..8b1d95561 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/DynamicPropertyAggregationFormula.java +++ b/src/main/java/io/ebeaninternal/server/deploy/DynamicPropertyAggregationFormula.java @@ -1,9 +1,11 @@ package io.ebeaninternal.server.deploy; import io.ebeaninternal.server.query.SqlBeanLoad; +import io.ebeaninternal.server.type.DataReader; import io.ebeaninternal.server.type.ScalarType; import javax.persistence.PersistenceException; +import java.sql.SQLException; /** * Dynamic property based on aggregation (max, min, avg, count). @@ -36,6 +38,15 @@ class DynamicPropertyAggregationFormula extends DynamicPropertyBase { return aggregate; } + @Override + public Object read(DataReader dataReader) throws SQLException { + try { + return scalarType.read(dataReader); + } catch (Exception e) { + throw new PersistenceException("Error loading on " + fullName, e); + } + } + @Override public void load(SqlBeanLoad sqlBeanLoad) { diff --git a/src/main/java/io/ebeaninternal/server/query/CQueryFetchSingleAttribute.java b/src/main/java/io/ebeaninternal/server/query/CQueryFetchSingleAttribute.java index 3b8b8ef01..061a35b71 100644 --- a/src/main/java/io/ebeaninternal/server/query/CQueryFetchSingleAttribute.java +++ b/src/main/java/io/ebeaninternal/server/query/CQueryFetchSingleAttribute.java @@ -1,14 +1,14 @@ package io.ebeaninternal.server.query; -import io.ebean.util.JdbcClose; import io.ebean.CountedValue; +import io.ebean.util.JdbcClose; import io.ebeaninternal.api.SpiProfileTransactionEvent; import io.ebeaninternal.api.SpiQuery; import io.ebeaninternal.api.SpiTransaction; import io.ebeaninternal.server.core.OrmQueryRequest; import io.ebeaninternal.server.deploy.BeanDescriptor; import io.ebeaninternal.server.type.RsetDataReader; -import io.ebeaninternal.server.type.ScalarType; +import io.ebeaninternal.server.type.ScalarDataReader; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -60,7 +60,7 @@ class CQueryFetchSingleAttribute implements SpiProfileTransactionEvent { private int rowCount; - private final ScalarType scalarType; + private final ScalarDataReader reader; private final boolean containsCounts; @@ -77,7 +77,7 @@ class CQueryFetchSingleAttribute implements SpiProfileTransactionEvent { this.desc = request.getBeanDescriptor(); this.predicates = predicates; this.containsCounts = containsCounts; - this.scalarType = queryPlan.getSingleAttributeScalarType(); + this.reader = queryPlan.getSingleAttributeScalarType(); query.setGeneratedSql(sql); } @@ -106,7 +106,7 @@ class CQueryFetchSingleAttribute implements SpiProfileTransactionEvent { List result = new ArrayList<>(); while (dataReader.next()) { - Object value = scalarType.read(dataReader); + Object value = reader.read(dataReader); if (containsCounts) { value = new CountedValue<>(value, dataReader.getLong()); } diff --git a/src/main/java/io/ebeaninternal/server/query/CQueryPlan.java b/src/main/java/io/ebeaninternal/server/query/CQueryPlan.java index 7b4b27e18..657a2bdcc 100644 --- a/src/main/java/io/ebeaninternal/server/query/CQueryPlan.java +++ b/src/main/java/io/ebeaninternal/server/query/CQueryPlan.java @@ -15,7 +15,7 @@ import io.ebeaninternal.server.query.CQueryPlanStats.Snapshot; import io.ebeaninternal.server.type.DataBind; import io.ebeaninternal.server.type.DataReader; import io.ebeaninternal.server.type.RsetDataReader; -import io.ebeaninternal.server.type.ScalarType; +import io.ebeaninternal.server.type.ScalarDataReader; import io.ebeaninternal.server.util.Md5; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -286,8 +286,8 @@ public class CQueryPlan { return stats.getLastQueryTime(); } - ScalarType getSingleAttributeScalarType() { - return sqlTree.getRootNode().getSingleAttributeScalarType(); + ScalarDataReader getSingleAttributeScalarType() { + return sqlTree.getRootNode().getSingleAttributeReader(); } /** diff --git a/src/main/java/io/ebeaninternal/server/query/STreeProperty.java b/src/main/java/io/ebeaninternal/server/query/STreeProperty.java index 340766e9a..b0c2ee916 100644 --- a/src/main/java/io/ebeaninternal/server/query/STreeProperty.java +++ b/src/main/java/io/ebeaninternal/server/query/STreeProperty.java @@ -2,6 +2,7 @@ package io.ebeaninternal.server.query; import io.ebeaninternal.server.deploy.DbReadContext; import io.ebeaninternal.server.deploy.DbSqlContext; +import io.ebeaninternal.server.type.ScalarDataReader; import io.ebeaninternal.server.type.ScalarType; import java.util.List; @@ -11,7 +12,7 @@ import java.util.List; *

* A BeanProperty or a dynamically created property based on formula. */ -public interface STreeProperty { +public interface STreeProperty extends ScalarDataReader { /** * Return the property name. diff --git a/src/main/java/io/ebeaninternal/server/query/STreePropertyAssocOne.java b/src/main/java/io/ebeaninternal/server/query/STreePropertyAssocOne.java index 4ec7bcbc5..c7d9c8f85 100644 --- a/src/main/java/io/ebeaninternal/server/query/STreePropertyAssocOne.java +++ b/src/main/java/io/ebeaninternal/server/query/STreePropertyAssocOne.java @@ -1,7 +1,7 @@ package io.ebeaninternal.server.query; import io.ebean.bean.EntityBean; -import io.ebeaninternal.server.type.ScalarType; +import io.ebeaninternal.server.type.ScalarDataReader; public interface STreePropertyAssocOne extends STreePropertyAssoc { @@ -13,7 +13,7 @@ public interface STreePropertyAssocOne extends STreePropertyAssoc { /** * Return the scalar type of the associated id property. */ - ScalarType getIdScalarType(); + ScalarDataReader getIdReader(); /** * Returns true, if this relation has a foreign key. diff --git a/src/main/java/io/ebeaninternal/server/query/SqlTreeNode.java b/src/main/java/io/ebeaninternal/server/query/SqlTreeNode.java index 39e0bff28..c78de34a5 100644 --- a/src/main/java/io/ebeaninternal/server/query/SqlTreeNode.java +++ b/src/main/java/io/ebeaninternal/server/query/SqlTreeNode.java @@ -5,7 +5,7 @@ import io.ebean.bean.EntityBean; import io.ebeaninternal.api.SpiQuery; import io.ebeaninternal.server.deploy.DbReadContext; import io.ebeaninternal.server.deploy.DbSqlContext; -import io.ebeaninternal.server.type.ScalarType; +import io.ebeaninternal.server.type.ScalarDataReader; import java.sql.SQLException; import java.util.List; @@ -80,9 +80,9 @@ interface SqlTreeNode { boolean hasMany(); /** - * Return the property for singleAttribute query. + * Return the reader for the single attribute query. */ - ScalarType getSingleAttributeScalarType(); + ScalarDataReader getSingleAttributeReader(); /** * Return true if the query is known to only have a single property selected. diff --git a/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeBean.java b/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeBean.java index 73ffb3e23..a62106a52 100644 --- a/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeBean.java +++ b/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeBean.java @@ -14,7 +14,7 @@ import io.ebeaninternal.server.deploy.DbSqlContext; import io.ebeaninternal.server.deploy.InheritInfo; import io.ebeaninternal.server.deploy.TableJoin; import io.ebeaninternal.server.deploy.id.IdBinder; -import io.ebeaninternal.server.type.ScalarType; +import io.ebeaninternal.server.type.ScalarDataReader; import java.sql.SQLException; import java.sql.Timestamp; @@ -145,22 +145,22 @@ class SqlTreeNodeBean implements SqlTreeNode { } @Override - public ScalarType getSingleAttributeScalarType() { + public ScalarDataReader getSingleAttributeReader() { if (properties == null || properties.length == 0) { // if we have no property ask first children (in a distinct select with join) if (children.length == 0) { // expected to be a findIds query - return desc.getIdBinder().getBeanProperty().getScalarType(); + return desc.getIdBinder().getBeanProperty(); } - return children[0].getSingleAttributeScalarType(); + return children[0].getSingleAttributeReader(); } if (properties[0] instanceof STreePropertyAssocOne) { STreePropertyAssocOne assocOne = (STreePropertyAssocOne)properties[0]; if (assocOne.isAssocId()) { - return assocOne.getIdScalarType(); + return assocOne.getIdReader(); } } - return properties[0].getScalarType(); + return properties[0]; } private Map createPathMap(String prefix, STreeType desc) { diff --git a/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeExtraJoin.java b/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeExtraJoin.java index 6dfd1fd7b..f1df4476d 100644 --- a/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeExtraJoin.java +++ b/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeExtraJoin.java @@ -76,7 +76,7 @@ class SqlTreeNodeExtraJoin implements SqlTreeNode { } @Override - public ScalarType getSingleAttributeScalarType() { + public ScalarType getSingleAttributeReader() { throw new IllegalStateException("No expected"); } diff --git a/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeManyWhereJoin.java b/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeManyWhereJoin.java index a5cd0e232..1db89c56f 100644 --- a/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeManyWhereJoin.java +++ b/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeManyWhereJoin.java @@ -43,7 +43,7 @@ class SqlTreeNodeManyWhereJoin implements SqlTreeNode { } @Override - public ScalarType getSingleAttributeScalarType() { + public ScalarType getSingleAttributeReader() { throw new IllegalStateException("No expected"); } diff --git a/src/main/java/io/ebeaninternal/server/type/ScalarDataReader.java b/src/main/java/io/ebeaninternal/server/type/ScalarDataReader.java index 00787b823..f410cbe09 100644 --- a/src/main/java/io/ebeaninternal/server/type/ScalarDataReader.java +++ b/src/main/java/io/ebeaninternal/server/type/ScalarDataReader.java @@ -12,14 +12,4 @@ public interface ScalarDataReader { */ T read(DataReader dataReader) throws SQLException; - /** - * Ignore typically by moving the index position. - */ - void loadIgnore(DataReader dataReader); - - /** - * Bind the value to the underlying preparedStatement. - */ - void bind(DataBind b, T value) throws SQLException; - } diff --git a/src/main/java/io/ebeaninternal/server/type/ScalarType.java b/src/main/java/io/ebeaninternal/server/type/ScalarType.java index e8e609a73..a61b8ac2b 100644 --- a/src/main/java/io/ebeaninternal/server/type/ScalarType.java +++ b/src/main/java/io/ebeaninternal/server/type/ScalarType.java @@ -1,10 +1,10 @@ package io.ebeaninternal.server.type; +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.core.JsonParser; import io.ebean.text.StringFormatter; import io.ebean.text.StringParser; import io.ebeanservice.docstore.api.mapping.DocPropertyType; -import com.fasterxml.jackson.core.JsonGenerator; -import com.fasterxml.jackson.core.JsonParser; import java.io.DataInput; import java.io.DataOutput; @@ -103,7 +103,6 @@ public interface ScalarType extends StringParser, StringFormatter, ScalarData * Ignore the reading of this value. Typically this means moving the index * position in the ResultSet. */ - @Override void loadIgnore(DataReader reader); /** @@ -113,7 +112,6 @@ public interface ScalarType extends StringParser, StringFormatter, ScalarData * JDBC type. *

*/ - @Override void bind(DataBind bind, T value) throws SQLException; /** diff --git a/src/test/java/org/tests/compositekeys/TestCKeyDelete.java b/src/test/java/org/tests/compositekeys/TestCKeyDelete.java index f25d44c0a..c3697e043 100644 --- a/src/test/java/org/tests/compositekeys/TestCKeyDelete.java +++ b/src/test/java/org/tests/compositekeys/TestCKeyDelete.java @@ -2,10 +2,15 @@ package org.tests.compositekeys; import io.ebean.BaseTestCase; import io.ebean.Ebean; +import org.junit.Test; import org.tests.model.basic.CKeyParent; import org.tests.model.basic.CKeyParentId; -import org.junit.Assert; -import org.junit.Test; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; public class TestCKeyDelete extends BaseTestCase { @@ -22,22 +27,20 @@ public class TestCKeyDelete extends BaseTestCase { Ebean.save(p); CKeyParent found = Ebean.find(CKeyParent.class).where().idEq(searchId).findOne(); - - Assert.assertNotNull(found); + assertNotNull(found); Ebean.delete(CKeyParent.class, searchId); CKeyParent notFound = Ebean.find(CKeyParent.class).where().idEq(searchId).findOne(); - - Assert.assertNull(notFound); + assertNull(notFound); } @Test public void testDeleteWhere() { - CKeyParentId id = new CKeyParentId(100, "deleteMe"); - CKeyParentId searchId = new CKeyParentId(100, "deleteMe"); + CKeyParentId id = new CKeyParentId(101, "deleteMe2"); + CKeyParentId searchId = new CKeyParentId(101, "deleteMe2"); CKeyParent p = new CKeyParent(); p.setId(id); @@ -45,10 +48,16 @@ public class TestCKeyDelete extends BaseTestCase { Ebean.save(p); - Ebean.createQuery(CKeyParent.class).where().eq("id.oneKey", 100).delete(); + List ids = Ebean.find(CKeyParent.class).where().eq("id.oneKey", 101).findIds(); + assertThat(ids).hasSize(1); + + CKeyParentId foundId = ids.get(0); + assertThat(foundId.getOneKey()).isEqualTo(101); + assertThat(foundId.getTwoKey()).isEqualTo("deleteMe2"); + + Ebean.createQuery(CKeyParent.class).where().eq("id.oneKey", 101).delete(); CKeyParent found = Ebean.find(CKeyParent.class).where().idEq(searchId).findOne(); - - Assert.assertNull(found); + assertNull(found); } }