Preserve extra data in RelationshipTarget with replace_related(_with_difference)

[urben1680]

Objective

The methods and commands replace_related and replace_related_with_difference may cause data stored at the RelationshipTarget be lost when all original children are removed before new children are added.

Part of #19589

Solution

Fix the issue, either by removing the old children after adding the new ones and not before (replace_related_with_difference) or by taking the whole RelationshipTarget to modify it, not only the inner collection (replace_related).

Testing

I added a new test asserting the data is kept. I also added a general test of these methods as they had none previously.

[alice-i-cecile]

Good; thanks for the tests here. This is too tricky to be confident without them.

[gwafotapa]

Looks good to me! I would just like to see the dead code disappear.

On a side note, I'm not sure why replace_related_with_difference needs the entities_to_relate argument.

[urben1680]

On a side note, I'm not sure why replace_related_with_difference needs the entities_to_relate argument.

It is more efficient to reinsert them after clearing the RelationshipTarget. Otherwise you would have to call RelationshipSourceCollection::remove in a loop which is quite inefficient for Vec<Entity>. It is a better idea to just insert them again, also because the typical Vec<Entity> can be easily extended via a slice.

Also you need it in case the RelationshipTarget is missing despite the caller's assumption. If the first reason was not present, this would have to panic in that case.

Good call on the branches. 👍

[gwafotapa]

Ah I see. Thank you.