Fix "The revision_user revision metadata key is not set." and friends

Ha, 1 test fail is your definition of a spectacular fail? ;)

Looks nice. The thing is that entity_test is a non-revisionable entity type, so it doesn't need revision metadata. And that entity type is used in dozens if not hundreds of tests and only a single one fails with deprecation messages, whch makes me wonder whether that test is doing something weird.

Looked at one of the failing test methods, it installs entity_schema_test and that has entity_schema_test_entity_type_alter() and makes the entity_test entity type, which by default is not revisionable exactly that. So we need to update that hook to also set the metadata keys an we should be done here.

There were two problems with the changes.

a) You cleared caches on the wrong service, there are multiple places with caches that were split off from entity manager. What we need here is the entity_type.manager cache because we're changing what the entity type alter hook returns. With that fixed, there's no difference anymore on entity.manager vs. specific services.

b) The metadata needs to match the fields that actually exist. Those fields are not all required, only those that exist and in this case, entity_schema_test_entity_base_field_info() only adds the revision_log field. Having those metadata keys set to fields that didn't exist caused an exception that was caught internally but then the storage was in a messed up state. That's not pretty but it's not up to this issue to change that.

Also added a sentence about those keys to the documention page you added a @todo for and removed that again.

+++ b/core/lib/Drupal/Core/Entity/ContentEntityType.php
@@ -88,15 +88,15 @@ public function getRevisionMetadataKeys($include_backwards_compatibility_field_n
+        @trigger_error('The revision_user revision metadata key is not set for entity type: ' . $this->id, E_USER_DEPRECATED);
...
+        @trigger_error('The revision_created revision metadata key is not set for entity type: ' . $this->id, E_USER_DEPRECATED);
...
+        @trigger_error('The revision_log_message revision metadata key is not set for entity type: ' . $this->id, E_USER_DEPRECATED);

should we be adding a link to a Change record while we're here?

1. +++ b/core/modules/system/tests/modules/entity_schema_test/entity_schema_test.module
   @@ -14,13 +15,24 @@
   +    /* @var $entity_type \Drupal\Core\Entity\ContentEntityTypeInterface */
   

   We usually put the class name before the variable name in inline helpers like this. Also, we're missing a * before @var :)

2. +++ b/core/tests/Drupal/KernelTests/Core/Entity/EntitySchemaTest.php
   @@ -66,11 +66,12 @@ public function testCustomFieldCreateDelete() {
   -    $original = $this->entityManager->getDefinition($entity_test_id);
   -    $this->entityManager->clearCachedDefinitions();
   +    $entity_type_manager = $this->container->get('entity_type.manager');
   +    $original = $entity_type_manager->getDefinition($entity_test_id);
   +    $entity_type_manager->clearCachedDefinitions();
   ...
   -    $entity_type = $this->entityManager->getDefinition($entity_test_id);
   -    $this->entityManager->onEntityTypeUpdate($entity_type, $original);
   +    $entity_type = $entity_type_manager->getDefinition($entity_test_id);
   +    $this->container->get('entity_type.listener')->onEntityTypeUpdate($entity_type, $original);
   

   Are these changes really required for this patch?

1. Yep, that line is bogus, also not necessary as you explicitly added the instanceof, that does the same for the IDE.
2. No, it's not.

Nice, looks ready to me :)

Committed d4437e2 and pushed to 8.7.x. Thanks!