Cloning an entity with initialized translations leaves $entityKeys and $translatableEntityKeys pointing to the old entity object

Here's an initial patch, just so I can include a reference to the patch in our project.

Will add docs + test and also try to figure out if any more things are affected by the same problem.

Hah, so this is actually postponed on #2675010: Cloned entity will point to the same field objects if the clone was created after an entity translation has been initialized because it cannot be properly tested otherwise as ContentEntityBase::isNewRevision() also checks ContentEntityBase::getRevisionId() and the field value of the revision ID gets unset in the clone and because of that issue it is also unset in the original.

Attached patch should fail and will pass once that is in.

Also in terms of other things that need updating, I checked...

Anything is made a reference in ContentEntityBase::initializeTranslation() is potentially problematic. This is the list:

    $translation->translations = &$this->translations;
    $translation->enforceIsNew = &$this->enforceIsNew;

-> Already covered

    $translation->fields = &$this->fields;

-> Handled by #2675010: Cloned entity will point to the same field objects if the clone was created after an entity translation has been initialized

    $translation->newRevision = &$this->newRevision;

-> Handled here

    $translation->values = &$this->values;
    $translation->entityKeys = &$this->entityKeys;
    $translation->translatableEntityKeys = &$this->translatableEntityKeys;

-> These are not handled yet.

So we should open a new issue for that. That will also be postponed on #2675010: Cloned entity will point to the same field objects if the clone was created after an entity translation has been initialized, though. Because all of those properties are "derivative" properties of $this->fields. I would still like to keep the current scope of this issue as this fixes an actually reproducible bug, whereas the others fix potential bugs, but there are no "known bugs" at the moment with them.

Hah, that's nice. I was about to post a comment, saying that $this->values is not problematic because it's never actually updated, but you are - as always - two steps ahead of me. I don't even want to contemplate the headache that will cause...

OK, so now that #2675010: Cloned entity will point to the same field objects if the clone was created after an entity translation has been initialized is taking care of the revision as well, let's repurpose to the other properties found in #12

Still postponed on that.

Thanks!! Looks great to me, I just have one quibble with the test before marking it RTBC.

+++ b/core/tests/Drupal/KernelTests/Core/Entity/ContentEntityCloneTest.php
@@ -179,4 +179,41 @@ public function testNewRevisionOnCloneEntityTranslation() {
+      'name' => $this->randomString(),
...
+    $clone->$label_field_name->value = 'test-change-label';

Let's not use a random string here and compare that with a fixed string. Even if they will never be equal (due to the different length) this makes the test a bit harder to understand than necessary. Let's just use a fixed string in both places.

Same could be said about the UUID, actually, so maybe we can just use fixed values there as well.

Perfect, thank you!

Note: I did upload some patches earlier in this issue, but that was when this issue had a related but completely distinct scope, so I did not myself work on any of the code in the latest patch.