Problem/Motivation
This issue is similar to #2772979: Enforcing a cloned entity translation to be new propagates to the original entity and #2828073: Cloning an entity with initialized translations leaves $entityKeys and $translatableEntityKeys pointing to the old entity object and the problem has been initially mentioned in #2828073: Cloning an entity with initialized translations leaves $entityKeys and $translatableEntityKeys pointing to the old entity object, but in order to keep each problem separately there is an isolated issue for the $values entity property.
What happens is that if an entity with at least one translation initialized is cloned then the $values property will be shared between the clone and the original entity and under circumstances the values of both entity will get mixed up. An example use is:
- Create an entity with at least one translation.
- Clone the entity or its translation.
- Alter a field value on the cloned entity object.
- Serialize the entity and the clone.
- Read out the value of the field that has been changed from the original entity.
- The returned value now is not the one from the original entity, but the altered one from the cloned entity object.
Proposed resolution
Break the reference of the property $values in CEB::__clone().
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#13 | 2862463-13.patch | 3.67 KB | jofitz |
#10 | interdiff-8-10.txt | 1.39 KB | hchonov |
#10 | 2862463-10.patch | 3.54 KB | hchonov |
#8 | interdiff-3-8.txt | 417 bytes | hchonov |
#8 | 2862463-8.patch | 2.66 KB | hchonov |
Comments
Comment #2
hchonovComment #3
hchonovAnd here the fix for the cloning problem.
Comment #4
hchonovComment #7
hchonovOuch, the problem was that we later use the $values variable and so overwrite the reference.. We have to either unset the $values variable or use different name for it. I think unsetting it should be fine.
Comment #8
hchonovOuhh.. somehow I've uploaded the previous patch by mistake instead the interdiff.. Here again.
Comment #9
tstoecklerSorry, but in my opinion this is pretty confusing because
$values
actually refers to different things both times.I'm not exactly sure what would be better naming, but I think we should avoid the
unset()
. Maybeforeach ($this->fields as $field)
instead? Not sure...Comment #10
hchonovWhat about
$fields_by_langcode
?Comment #11
tstoecklerI like it, thanks!
Comment #12
alexpottComment #13
jofitz CreditAttribution: jofitz at ComputerMinds commentedRe-rolled.
Comment #14
tstoecklerAwesome, thank you!
Comment #15
alexpottCommitted and pushed 1ef413b to 8.4.x and cc1c6b6 to 8.3.x. Thanks!