#2127 - Adding a new list of children to @OneToMany with orphanRemoval true does not delete existing orphans (#2131)

This commit is contained in:
Rob Bygrave
2020-12-16 15:36:44 +13:00
committed by GitHub
parent a132f09b03
commit 55324653a5
10 changed files with 86 additions and 53 deletions
@@ -73,13 +73,13 @@ public final class PersistRequestUpdateSql extends PersistRequest {
/**
* Add this request to BatchControl to flush later.
*/
public void addToFlushQueue(boolean early) {
public void addToFlushQueue(int pos) {
BatchControl control = transaction.getBatchControl();
if (control == null) {
control = persistExecute.createBatchControl(transaction);
}
flushQueue = true;
control.addToFlushQueue(this, early);
control.addToFlushQueue(this, pos);
}
@Override
@@ -104,18 +104,14 @@ public interface Persister {
void executeOrQueue(SpiSqlUpdate update, SpiTransaction t, boolean queue);
/**
* Queue the SqlUpdate for early execution (with JDBC batch).
* Queue the SqlUpdate for execution with position 0, 1 or 2 defining
* when it executes relative to the flush of beans .
*/
void addToFlushQueue(SpiSqlUpdate update, SpiTransaction t);
void addToFlushQueue(SpiSqlUpdate update, SpiTransaction t, int pos);
/**
* Queue the SqlUpdate for late execution (with JDBC batch).
* Add the statement to JDBC batch for later execution via executeBatch.
*/
void addToFlushQueueLast(SpiSqlUpdate update, SpiTransaction t);
/**
* Add the statement to JDBC batch for later execution via executeBatch.
*/
void addBatch(SpiSqlUpdate sqlUpdate, SpiTransaction transaction);
/**
@@ -81,8 +81,7 @@ public final class BatchControl {
*/
private int bufferMax;
private Queue earlyQueue;
private Queue lateQueue;
private Queue[] queues = new Queue[3];
/**
* Create for a given transaction, PersistExecute, default size and getGeneratedKeys.
@@ -271,9 +270,10 @@ public final class BatchControl {
}
private void flushBuffer(boolean reset) throws BatchedSqlException {
flushQueue(queues[0]);
flushInternal(reset);
flushQueue(earlyQueue);
flushQueue(lateQueue);
flushQueue(queues[1]);
flushQueue(queues[2]);
}
private void flushQueue(Queue queue) throws BatchedSqlException {
@@ -368,20 +368,11 @@ public final class BatchControl {
/**
* Add a SqlUpdate request to execute after flush.
*/
public void addToFlushQueue(PersistRequestUpdateSql request, boolean early) {
if (early) {
// add it to the early queue
if (earlyQueue == null) {
earlyQueue = new Queue();
}
earlyQueue.add(request);
} else {
// add it to the late queue
if (lateQueue == null) {
lateQueue = new Queue();
}
lateQueue.add(request);
public void addToFlushQueue(PersistRequestUpdateSql request, int pos) {
if (queues[pos] == null) {
queues[pos] = new Queue();
}
queues[pos].add(request);
}
private static class Queue {
@@ -141,24 +141,17 @@ public final class DefaultPersister implements Persister {
@Override
public void executeOrQueue(SpiSqlUpdate update, SpiTransaction t, boolean queue) {
if (queue) {
addToFlushQueue(update, t, false);
addToFlushQueue(update, t, 2);
} else {
executeSqlUpdate(update, t);
}
}
@Override
public void addToFlushQueue(SpiSqlUpdate update, SpiTransaction t) {
addToFlushQueue(update, t, true);
}
@Override
public void addToFlushQueueLast(SpiSqlUpdate update, SpiTransaction t) {
addToFlushQueue(update, t, false);
}
private void addToFlushQueue(SpiSqlUpdate update, SpiTransaction t, boolean early) {
new PersistRequestUpdateSql(server, update, t, persistExecute).addToFlushQueue(early);
/**
* Add to the flush queue in position 0, 1 or 2.
*/
public void addToFlushQueue(SpiSqlUpdate update, SpiTransaction t, int pos) {
new PersistRequestUpdateSql(server, update, t, persistExecute).addToFlushQueue(pos);
}
/**
@@ -963,7 +956,7 @@ public final class DefaultPersister implements Persister {
void deleteManyIntersection(EntityBean bean, BeanPropertyAssocMany<?> many, SpiTransaction t, boolean publish, boolean queue) {
SpiSqlUpdate sqlDelete = deleteAllIntersection(bean, many, publish);
if (queue) {
addToFlushQueue(sqlDelete, t, true);
addToFlushQueue(sqlDelete, t, 1);
} else {
executeSqlUpdate(sqlDelete, t);
}
@@ -1237,7 +1230,6 @@ public final class DefaultPersister implements Persister {
@SuppressWarnings({"unchecked"})
private <T> PersistRequestBean<T> createRequest(T bean, Transaction t, Object parentBean, BeanManager<?> mgr,
PersistRequest.Type type, int flags) {
// no delete requests come here
return new PersistRequestBean(server, bean, parentBean, mgr, (SpiTransaction) t, persistExecute, type, flags);
}
@@ -1252,7 +1244,6 @@ public final class DefaultPersister implements Persister {
@SuppressWarnings({"unchecked"})
private <T> PersistRequestBean<T> createDeleteRequest(Object bean, Transaction t, PersistRequest.Type type, int flags) {
BeanManager<T> mgr = getBeanManager(bean);
if (type == Type.DELETE_PERMANENT) {
type = Type.DELETE;
@@ -1281,7 +1272,6 @@ public final class DefaultPersister implements Persister {
*/
@SuppressWarnings("unchecked")
private <T> BeanManager<T> getBeanManager(Object bean) {
BeanManager<T> mgr = (BeanManager<T>) beanDescriptorManager.getBeanManager(bean.getClass());
if (mgr == null) {
throw new PersistenceException(errNotRegistered(bean.getClass()));
@@ -49,7 +49,7 @@ abstract class SaveManyBase implements SaveMany {
void preElementCollectionUpdate() {
if (!insertedParent) {
request.preElementCollectionUpdate();
persister.addToFlushQueue(many.deleteByParentId(request.getBeanId(), null), transaction);
persister.addToFlushQueue(many.deleteByParentId(request.getBeanId(), null), transaction, 1);
}
}
@@ -235,9 +235,6 @@ public class SaveManyBeans extends SaveManyBase {
/**
* Save the additions and removals from a ManyToMany collection as inserts
* and deletes from the intersection table.
* <p>
* This is done via MapBeans.
* </p>
*/
private void saveAssocManyIntersection() {
if (value == null) {
@@ -336,9 +333,14 @@ public class SaveManyBeans extends SaveManyBase {
}
private void removeAssocManyOrphans() {
// check that the list is not null and if it is a BeanCollection
// check that is has been populated (don't trigger lazy loading)
if (value instanceof BeanCollection<?>) {
if (value == null) {
return;
}
if (!(value instanceof BeanCollection<?>)) {
if (!insertedParent) {
persister.addToFlushQueue(many.deleteByParentId(request.getBeanId(), null), transaction, 0);
}
} else {
BeanCollection<?> c = (BeanCollection<?>) value;
Set<?> modifyRemovals = c.getModifyRemovals();
if (insertedParent) {
@@ -48,7 +48,7 @@ class SaveManyElementCollection extends SaveManyBase {
final SpiSqlUpdate sqlInsert = proto.copy();
sqlInsert.setParameter(parentId);
many.bindElementValue(sqlInsert, value);
persister.addToFlushQueueLast(sqlInsert, transaction);
persister.addToFlushQueue(sqlInsert, transaction, 2);
}
resetModifyState();
postElementCollectionUpdate();
@@ -51,7 +51,7 @@ class SaveManyElementCollectionMap extends SaveManyBase {
sqlInsert.setParameter(parentId);
sqlInsert.setParameter(entry.getKey());
many.bindElementValue(sqlInsert, entry.getValue());
persister.addToFlushQueueLast(sqlInsert, transaction);
persister.addToFlushQueue(sqlInsert, transaction, 2);
}
resetModifyState();
postElementCollectionUpdate();
@@ -34,4 +34,7 @@ public class COOne {
return children;
}
public void setChildren(List<COOneMany> children) {
this.children = children;
}
}
@@ -0,0 +1,51 @@
package org.tests.cascade;
import io.ebean.BaseTestCase;
import io.ebean.DB;
import org.junit.Test;
import java.util.ArrayList;
import java.util.List;
import static org.assertj.core.api.Assertions.assertThat;
public class TestDeleteO2MOrphans extends BaseTestCase {
@Test
public void test() {
final long id = setup();
// act
setNewChildren(id);
// assert
COOne check = findById(id);
assertThat(check.getChildren()).hasSize(2);
DB.delete(check);
}
private COOne findById(long id) {
return DB.find(COOne.class).where().idEq(id).findOne();
}
private void setNewChildren(long id) {
COOne found = findById(id);
found.setChildren(createManies("M3", "M4"));
DB.update(found);
}
private long setup() {
COOne company = new COOne("P0");
company.setChildren(createManies("M1", "M2"));
DB.insert(company);
return company.getId();
}
private List<COOneMany> createManies(String name1, String name2) {
COOneMany employee1 = new COOneMany(name1);
COOneMany employee2 = new COOneMany(name2);
List<COOneMany> employees = new ArrayList<>();
employees.add(employee1);
employees.add(employee2);
return employees;
}
}