diff --git a/src/main/java/com/avaje/ebean/Ebean.java b/src/main/java/com/avaje/ebean/Ebean.java index a0033877f..c478e90fe 100644 --- a/src/main/java/com/avaje/ebean/Ebean.java +++ b/src/main/java/com/avaje/ebean/Ebean.java @@ -697,12 +697,20 @@ public final class Ebean { /** * Delete the bean. *

+ * This will return true if the bean was deleted successfully or JDBC batch is being used. + *

+ *

* If there is no current transaction one will be created and committed for * you automatically. *

+ *

+ * If the Bean does not have a version property (or loaded version property) and + * the bean does not exist then this returns false indicating that nothing was + * deleted. Note that, if JDBC batch mode is used then this always returns true. + *

*/ - public static void delete(Object bean) throws OptimisticLockException { - serverMgr.getDefaultServer().delete(bean); + public static boolean delete(Object bean) throws OptimisticLockException { + return serverMgr.getDefaultServer().delete(bean); } /** diff --git a/src/main/java/com/avaje/ebean/EbeanServer.java b/src/main/java/com/avaje/ebean/EbeanServer.java index 4098ad870..49ca50f0a 100644 --- a/src/main/java/com/avaje/ebean/EbeanServer.java +++ b/src/main/java/com/avaje/ebean/EbeanServer.java @@ -1246,16 +1246,32 @@ public interface EbeanServer { /** * Delete the bean. *

+ * This will return true if the bean was deleted successfully or JDBC batch is being used. + *

+ *

* If there is no current transaction one will be created and committed for * you automatically. *

+ *

+ * If the Bean does not have a version property (or loaded version property) and + * the bean does not exist then this returns false indicating that nothing was + * deleted. Note that, if JDBC batch mode is used then this always returns true. + *

*/ - void delete(Object bean) throws OptimisticLockException; + boolean delete(Object bean) throws OptimisticLockException; /** * Delete the bean with an explicit transaction. + *

+ * This will return true if the bean was deleted successfully or JDBC batch is being used. + *

+ *

+ * If the Bean does not have a version property (or loaded version property) and + * the bean does not exist then this returns false indicating that nothing was + * deleted. However, if JDBC batch mode is used then this always returns true. + *

*/ - void delete(Object bean, Transaction transaction) throws OptimisticLockException; + boolean delete(Object bean, Transaction transaction) throws OptimisticLockException; /** * Delete the bean given its type and id. diff --git a/src/main/java/com/avaje/ebean/Model.java b/src/main/java/com/avaje/ebean/Model.java index 49c9f0d63..50dffb060 100644 --- a/src/main/java/com/avaje/ebean/Model.java +++ b/src/main/java/com/avaje/ebean/Model.java @@ -251,12 +251,24 @@ public abstract class Model { } /** - * Delete this entity. + * Delete this bean. + *

+ * This will return true if the bean was deleted successfully or JDBC batch is being used. + *

+ *

+ * If there is no current transaction one will be created and committed for + * you automatically. + *

+ *

+ * If the Bean does not have a version property (or loaded version property) and + * the bean does not exist then this returns false indicating that nothing was + * deleted. Note that, if JDBC batch mode is used then this always returns true. + *

* * @see EbeanServer#delete(Object) */ - public void delete() { - db().delete(this); + public boolean delete() { + return db().delete(this); } /** @@ -276,8 +288,8 @@ public abstract class Model { /** * Perform a delete using this entity against the specified server. */ - public void delete(String server) { - db(server).delete(this); + public boolean delete(String server) { + return db(server).delete(this); } /** diff --git a/src/main/java/com/avaje/ebeaninternal/server/core/DefaultServer.java b/src/main/java/com/avaje/ebeaninternal/server/core/DefaultServer.java index 3be4ddb05..c6e25515c 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/core/DefaultServer.java +++ b/src/main/java/com/avaje/ebeaninternal/server/core/DefaultServer.java @@ -1867,16 +1867,16 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer { /** * Delete the bean. */ - public void delete(Object bean) { - delete(bean, null); + public boolean delete(Object bean) { + return delete(bean, null); } /** * Delete the bean with the explicit transaction. */ - public void delete(Object bean, Transaction t) { + public boolean delete(Object bean, Transaction t) { - persister.delete(checkEntityBean(bean), t); + return persister.delete(checkEntityBean(bean), t); } /** diff --git a/src/main/java/com/avaje/ebeaninternal/server/core/PersistRequestBean.java b/src/main/java/com/avaje/ebeaninternal/server/core/PersistRequestBean.java index ec1d90757..38082f626 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/core/PersistRequestBean.java +++ b/src/main/java/com/avaje/ebeaninternal/server/core/PersistRequestBean.java @@ -480,8 +480,7 @@ public final class PersistRequestBean extends PersistRequest implements BeanP return -1; case DELETE: - persistExecute.executeDeleteBean(this); - return -1; + return persistExecute.executeDeleteBean(this); default: throw new RuntimeException("Invalid type " + type); diff --git a/src/main/java/com/avaje/ebeaninternal/server/core/Persister.java b/src/main/java/com/avaje/ebeaninternal/server/core/Persister.java index a1e08d839..df79b7ae2 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/core/Persister.java +++ b/src/main/java/com/avaje/ebeaninternal/server/core/Persister.java @@ -66,7 +66,7 @@ public interface Persister { /** * Delete the bean. */ - void delete(EntityBean entityBean, Transaction t); + boolean delete(EntityBean entityBean, Transaction t); /** * Delete multiple beans given a collection of Id values. diff --git a/src/main/java/com/avaje/ebeaninternal/server/persist/BeanPersister.java b/src/main/java/com/avaje/ebeaninternal/server/persist/BeanPersister.java index 3e3719025..96f75ab7f 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/persist/BeanPersister.java +++ b/src/main/java/com/avaje/ebeaninternal/server/persist/BeanPersister.java @@ -22,6 +22,6 @@ public interface BeanPersister { /** * execute the delete bean request. */ - void delete(PersistRequestBean request) throws PersistenceException; + int delete(PersistRequestBean request) throws PersistenceException; } diff --git a/src/main/java/com/avaje/ebeaninternal/server/persist/DefaultPersistExecute.java b/src/main/java/com/avaje/ebeaninternal/server/persist/DefaultPersistExecute.java index a72094e4f..a9f59bae7 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/persist/DefaultPersistExecute.java +++ b/src/main/java/com/avaje/ebeaninternal/server/persist/DefaultPersistExecute.java @@ -73,15 +73,17 @@ public final class DefaultPersistExecute implements PersistExecute { /** * execute the bean delete request. */ - public void executeDeleteBean(PersistRequestBean request) { + public int executeDeleteBean(PersistRequestBean request) { BeanManager mgr = request.getBeanManager(); BeanPersister persister = mgr.getBeanPersister(); BeanPersistController controller = request.getBeanController(); if (controller == null || controller.preDelete(request)) { - persister.delete(request); + return persister.delete(request); } + // delete handled by the BeanController so return 0 + return 0; } /** diff --git a/src/main/java/com/avaje/ebeaninternal/server/persist/DefaultPersister.java b/src/main/java/com/avaje/ebeaninternal/server/persist/DefaultPersister.java index f0ef43800..ea5968aed 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/persist/DefaultPersister.java +++ b/src/main/java/com/avaje/ebeaninternal/server/persist/DefaultPersister.java @@ -498,24 +498,25 @@ public final class DefaultPersister implements Persister { /** * Delete the bean with the explicit transaction. + * Return false if the delete is executed without OCC and 0 rows were deleted. */ - public void delete(EntityBean bean, Transaction t) { + public boolean delete(EntityBean bean, Transaction t) { PersistRequestBean request = createRequest(bean, t, Type.DELETE); - deleteRequest(request); + boolean deleted = deleteRequest(request); if (request.isDraftable()) { // we have just deleting a draft bean so now we need to delete the // associated 'live' bean. This is effectively an 'automatic publish'. - try { - deleteRequest(createRequest(request.createReference(), t, Type.DELETE, true)); - } catch (OptimisticLockException e) { - SUM.debug("Ignore OptimisticLockException - did not delete live row as draft not published for bean:{} id:{}", request.getFullName(), request.getBeanId()); - } + deleteRequest(createRequest(request.createReference(), t, Type.DELETE, true)); } + return deleted; } - private void deleteRequest(PersistRequestBean req) { + /** + * Execute the delete request returning true if a delete occurred. + */ + private boolean deleteRequest(PersistRequestBean req) { if (req.isRegisteredForDeleteBean()) { // skip deleting bean. Used where cascade is on @@ -523,15 +524,17 @@ public final class DefaultPersister implements Persister { if (logger.isDebugEnabled()) { logger.debug("skipping delete on alreadyRegistered " + req.getBean()); } - return; + return false; } try { req.initTransIfRequiredWithBatchCascade(); - delete(req); + boolean deleted = delete(req); req.commitTransIfRequired(); req.flushBatchOnCascade(); + return deleted; + } catch (RuntimeException ex) { req.rollbackTransIfRequired(); throw ex; @@ -710,7 +713,7 @@ public final class DefaultPersister implements Persister { * Note that preDelete fires before the deletion of children. *

*/ - private void delete(PersistRequestBean request) { + private boolean delete(PersistRequestBean request) { DeleteUnloadedForeignKeys unloadedForeignKeys = null; @@ -729,7 +732,7 @@ public final class DefaultPersister implements Persister { } } - request.executeOrQueue(); + int count = request.executeOrQueue(); if (request.isPersistCascade()) { deleteAssocOne(request); @@ -739,6 +742,8 @@ public final class DefaultPersister implements Persister { } } + // return true if using JDBC batch (as we can't tell until the batch is flushed) + return count != 0; } /** diff --git a/src/main/java/com/avaje/ebeaninternal/server/persist/PersistExecute.java b/src/main/java/com/avaje/ebeaninternal/server/persist/PersistExecute.java index 6cc2e1fcc..c0183aa4b 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/persist/PersistExecute.java +++ b/src/main/java/com/avaje/ebeaninternal/server/persist/PersistExecute.java @@ -33,7 +33,7 @@ public interface PersistExecute { /** * Execute a Bean (or MapBean) delete. */ - void executeDeleteBean(PersistRequestBean request); + int executeDeleteBean(PersistRequestBean request); /** * Execute a Update. diff --git a/src/main/java/com/avaje/ebeaninternal/server/persist/dml/DeleteHandler.java b/src/main/java/com/avaje/ebeaninternal/server/persist/dml/DeleteHandler.java index 5dee3339b..9d5991c9c 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/persist/dml/DeleteHandler.java +++ b/src/main/java/com/avaje/ebeaninternal/server/persist/dml/DeleteHandler.java @@ -46,9 +46,10 @@ public class DeleteHandler extends DmlHandler { * Execute the delete non-batch. */ @Override - public void execute() throws SQLException, OptimisticLockException { + public int execute() throws SQLException, OptimisticLockException { int rowCount = dataBind.executeUpdate(); checkRowCount(rowCount); + return rowCount; } @Override diff --git a/src/main/java/com/avaje/ebeaninternal/server/persist/dml/DmlBeanPersister.java b/src/main/java/com/avaje/ebeaninternal/server/persist/dml/DmlBeanPersister.java index ba1faeffe..737e8000a 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/persist/dml/DmlBeanPersister.java +++ b/src/main/java/com/avaje/ebeaninternal/server/persist/dml/DmlBeanPersister.java @@ -35,10 +35,10 @@ public final class DmlBeanPersister implements BeanPersister { /** * execute the bean delete request. */ - public void delete(PersistRequestBean request) { + public int delete(PersistRequestBean request) { DeleteHandler delete = new DeleteHandler(request, deleteMeta); - execute(request, delete); + return execute(request, delete); } /** @@ -62,15 +62,17 @@ public final class DmlBeanPersister implements BeanPersister { /** * execute request taking batching into account. */ - private void execute(PersistRequestBean request, PersistHandler handler) { + private int execute(PersistRequestBean request, PersistHandler handler) { boolean batched = request.isBatched(); try { handler.bind(); if (batched) { handler.addBatch(); + return -1; + } else { - handler.execute(); + return handler.execute(); } } catch (SQLException e) { diff --git a/src/main/java/com/avaje/ebeaninternal/server/persist/dml/DmlHandler.java b/src/main/java/com/avaje/ebeaninternal/server/persist/dml/DmlHandler.java index 782165766..66d1365c0 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/persist/dml/DmlHandler.java +++ b/src/main/java/com/avaje/ebeaninternal/server/persist/dml/DmlHandler.java @@ -75,7 +75,7 @@ public abstract class DmlHandler implements PersistHandler, BindableRequest { * Execute now for non-batch execution. */ @Override - public abstract void execute() throws SQLException; + public abstract int execute() throws SQLException; /** * Check the rowCount. diff --git a/src/main/java/com/avaje/ebeaninternal/server/persist/dml/InsertHandler.java b/src/main/java/com/avaje/ebeaninternal/server/persist/dml/InsertHandler.java index 1525ca479..386edec12 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/persist/dml/InsertHandler.java +++ b/src/main/java/com/avaje/ebeaninternal/server/persist/dml/InsertHandler.java @@ -125,8 +125,8 @@ public class InsertHandler extends DmlHandler { * getGeneratedKeys if required. */ @Override - public void execute() throws SQLException, OptimisticLockException { - int rc = dataBind.executeUpdate(); + public int execute() throws SQLException, OptimisticLockException { + int rowCount = dataBind.executeUpdate(); if (useGeneratedKeys) { // get the auto-increment value back and set into the bean getGeneratedKeys(); @@ -136,8 +136,9 @@ public class InsertHandler extends DmlHandler { fetchGeneratedKeyUsingSelect(); } - checkRowCount(rc); + checkRowCount(rowCount); executeDerivedRelationships(); + return rowCount; } protected void executeDerivedRelationships() { @@ -210,16 +211,14 @@ public class InsertHandler extends DmlHandler { rset.close(); } } catch (SQLException ex) { - String msg = "Error closing rset for fetchGeneratedKeyUsingSelect?"; - logger.warn(msg, ex); + logger.warn("Error closing ResultSet for fetchGeneratedKeyUsingSelect?", ex); } try { if (stmt != null) { stmt.close(); } } catch (SQLException ex) { - String msg = "Error closing stmt for fetchGeneratedKeyUsingSelect?"; - logger.warn(msg, ex); + logger.warn("Error closing Statement for fetchGeneratedKeyUsingSelect?", ex); } } } diff --git a/src/main/java/com/avaje/ebeaninternal/server/persist/dml/PersistHandler.java b/src/main/java/com/avaje/ebeaninternal/server/persist/dml/PersistHandler.java index 0731d5351..1bd238791 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/persist/dml/PersistHandler.java +++ b/src/main/java/com/avaje/ebeaninternal/server/persist/dml/PersistHandler.java @@ -25,7 +25,7 @@ public interface PersistHandler { /** * Execute now for non-batch execution. */ - void execute() throws SQLException; + int execute() throws SQLException; /** * Close resources including underlying preparedStatement. diff --git a/src/main/java/com/avaje/ebeaninternal/server/persist/dml/UpdateHandler.java b/src/main/java/com/avaje/ebeaninternal/server/persist/dml/UpdateHandler.java index 3da51254f..388331753 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/persist/dml/UpdateHandler.java +++ b/src/main/java/com/avaje/ebeaninternal/server/persist/dml/UpdateHandler.java @@ -67,11 +67,13 @@ public class UpdateHandler extends DmlHandler { * Execute the update in non-batch. */ @Override - public void execute() throws SQLException, OptimisticLockException { + public int execute() throws SQLException, OptimisticLockException { if (!emptySetClause) { int rowCount = dataBind.executeUpdate(); checkRowCount(rowCount); + return rowCount; } + return 0; } @Override diff --git a/src/test/java/com/avaje/ebeaninternal/api/TDSpiEbeanServer.java b/src/test/java/com/avaje/ebeaninternal/api/TDSpiEbeanServer.java index 79bf7ad8a..dd9750044 100644 --- a/src/test/java/com/avaje/ebeaninternal/api/TDSpiEbeanServer.java +++ b/src/test/java/com/avaje/ebeaninternal/api/TDSpiEbeanServer.java @@ -548,8 +548,8 @@ public class TDSpiEbeanServer implements SpiEbeanServer { } @Override - public void delete(Object bean) throws OptimisticLockException { - + public boolean delete(Object bean) throws OptimisticLockException { + return false; } @Override @@ -658,8 +658,8 @@ public class TDSpiEbeanServer implements SpiEbeanServer { } @Override - public void delete(Object bean, Transaction t) throws OptimisticLockException { - + public boolean delete(Object bean, Transaction t) throws OptimisticLockException { + return false; } @Override diff --git a/src/test/java/com/avaje/tests/delete/TestDeleteWithoutOptimisticLocking.java b/src/test/java/com/avaje/tests/delete/TestDeleteWithoutOptimisticLocking.java index 0f5037c73..8487789db 100644 --- a/src/test/java/com/avaje/tests/delete/TestDeleteWithoutOptimisticLocking.java +++ b/src/test/java/com/avaje/tests/delete/TestDeleteWithoutOptimisticLocking.java @@ -2,19 +2,69 @@ package com.avaje.tests.delete; import com.avaje.ebean.BaseTestCase; import com.avaje.ebean.Ebean; +import com.avaje.ebean.EbeanServer; +import com.avaje.ebean.Transaction; +import com.avaje.ebean.config.PersistBatch; import com.avaje.tests.model.basic.Contact; +import com.avaje.tests.model.basic.EBasicVer; +import com.avaje.tests.model.converstation.Group; import org.junit.Test; +import static org.assertj.core.api.StrictAssertions.assertThat; + public class TestDeleteWithoutOptimisticLocking extends BaseTestCase { @Test - public void test() { + public void testSimpleBeanDelete_missingBean_returnsFalse() { // delete by by without version loaded ... should not throw OptimisticLockException Contact ref = Ebean.getReference(Contact.class, 999999); - Ebean.delete(ref); + assertThat(Ebean.delete(ref)).isFalse(); - Ebean.delete(Contact.class, 999999); + assertThat(Ebean.delete(Contact.class, 999999)).isEqualTo(0); + + // same as above but using Model.delete() + Group modelRef = Ebean.getReference(Group.class, 999999); + assertThat(modelRef.delete()).isFalse(); + } + + @Test + public void testSimpleBeanDelete_existingBean_returnsTrue() { + + EBasicVer basic = new EBasicVer(); + basic.setName("DelTest"); + Ebean.save(basic); + + EBasicVer basicRef = Ebean.getReference(EBasicVer.class, basic.getId()); + assertThat(Ebean.delete(basicRef)).isTrue(); + } + + + @Test + public void testSimpleBeanDelete_existingBeanWithJdbcBatch_returnsTrue() { + + EBasicVer basic = new EBasicVer(); + basic.setName("DelTestBatch"); + Ebean.save(basic); + + EbeanServer server = Ebean.getDefaultServer(); + Transaction transaction = server.beginTransaction(); + try { + transaction.setBatch(PersistBatch.ALL); + + // returns true even though the delete has not occurred yet + assertThat(server.delete(Ebean.getReference(EBasicVer.class, basic.getId()), transaction)).isTrue(); + + // returns true even though the bean does not exist + assertThat(server.delete(Ebean.getReference(EBasicVer.class, 999999), transaction)).isTrue(); + + transaction.commit(); + + assertThat(Ebean.find(EBasicVer.class, basic.getId())).isNull(); + + } finally { + transaction.end(); + } } }