set / clear and addAll does not work on a orphanRemoval=true

[tbee]

We have the following collection:

public class Expense {
    @OneToMany(orphanRemoval=true, mappedBy = "expense", cascade = CascadeType.ALL)
    protected List<ExpenseDistance> expenseDistances;
}

If a setter simply assigns the collection under EBean 12:

public void setExpenseDistances(List<ExpenseDistance> expenseDistances) {
        this.expenseDistances = expenseDistances;
}

Then we observe in some scenario's that existing expenseDistances are removed and only newly added are persisted. So we changed the implementation for collections with orhpanRemoval to:

    public void setExpenseDistances(List<ExpenseDistance> expenseDistances) {
        this.expenseDistances.clear();
        this.expenseDistances.addAll(expenseDistances);
    }

This results both under 7 and 12 in the behaviour that if a new collection with exactly the same entities is set (coming from a REST API), all expenseDistances are deleted. Somehow the orphan deletes are not negated by the new entities. Unoptimised we would at least have expected deletes followed by inserts.

The following does not work either:

    public void setExpenseDistances(List<ExpenseDistance> expenseDistances) {
        this.expenseDistances.addAll(expenseDistances);
        this.expenseDistances.retainAll(expenseDistances);
    }

This code below seems to work correctly:

    public void setExpenseDistances(List<ExpenseDistance> expenseDistances) {
        // remove those that no longer exist
        for (Iterator<ExpenseDistance> iterator = this.expenseDistances.iterator(); iterator.hasNext();) { // prevent concurrent modification
            ExpenseDistance expenseDistance = iterator.next();
            if (!expenseDistances.contains(expenseDistance)) {
                iterator.remove();
            }
        }
        // add those that are not present
        for (ExpenseDistance expenseDistance : expenseDistances) {
            if (!this.expenseDistances.contains(expenseDistance)) {
                this.expenseDistances.add(expenseDistance);
            }
        }
    }

[rbygrave]

This is an issue with List and how that is working specifically when orphan removal is used ( "Modify Listening" is enabled ). Being a List the add() and remove() default to work via instance rather that equals(). Changing this is going to be a behaviour change.

Next step is to organise the existing tests and add the new failing ones (that are expecting/needed equals to be used for adding and removing when orphan removal / modify listening is being used).

[tbee]

It turns out in Hibernate assigning a list with orphan removal in a setter is actively blocked (Hibernate throws an exception). That is logical because you need a List-implementation with orphan logic, so the original implementation of our setter simply is not okay. But the clear/addAll should work. As with the other issues, we will revisit this when we are at the latest EBean version. (We've just started using 12 into our code base.)

[rbygrave]

I've reviewed the tests and the current behaviour of List.clear() and List.addAll() and I think it does what its expected to.

Looking at the workaround code I think what is being asked for is for the List.addAll() behaviour to change to be like Set.addAll() and perform a contains check before adding an element.

The clear() + addAll() really must work as it does - I don't see much of a case for addAll there to do something like "restore" elements that were previously cleared.

I think the request here ends up as desiring List.retainAll() + List.addAll() to perform the same operation as the workaround code - effectively a change in behaviour for List.addAll().

[tbee]

We're rolling out J11 in phases to production ATM. It should be done in about 2 weeks. Then the first step is to update to EBean 13 and we'll pick up these issues again. And then we can do testing of any changes.