Issue #2938895 by plach, Wim Leers, hchonov, timmillwood: Make EntityConverter...

Issue #2938895 by plach, Wim Leers, hchonov, timmillwood: Make EntityConverter load the latest translation-affecting revision for translated entities

core/core.services.yml

@@ -965,7 +965,7 @@ services:
     class: Drupal\Core\ParamConverter\EntityConverter
     tags:
       - { name: paramconverter }
-    arguments: ['@entity.manager']
+    arguments: ['@entity.manager', '@language_manager']
   paramconverter.entity_revision:
     class: Drupal\Core\ParamConverter\EntityRevisionParamConverter
     tags:

core/lib/Drupal/Core/ParamConverter/EntityConverter.php

@@ -2,9 +2,12 @@
 
 namespace Drupal\Core\ParamConverter;
 
-use Drupal\Core\Entity\ContentEntityInterface;
 use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Entity\EntityManagerInterface;
+use Drupal\Core\Entity\RevisionableInterface;
+use Drupal\Core\Entity\TranslatableRevisionableInterface;
+use Drupal\Core\Language\LanguageInterface;
+use Drupal\Core\Language\LanguageManagerInterface;
 use Drupal\Core\TypedData\TranslatableInterface;
 use Symfony\Component\Routing\Route;
 
@@ -36,6 +39,29 @@
  *       example:
  *         type: entity:{entity_type}
  * @endcode
+ *
+ * If your route needs to support pending revisions, you can specify the
+ * "load_latest_revision" parameter. This will ensure that the latest revision
+ * is returned, even if it is not the default one:
+ * @code
+ * example.route:
+ *   path: foo/{example}
+ *   options:
+ *     parameters:
+ *       example:
+ *         type: entity:node
+ *         load_latest_revision: TRUE
+ * @endcode
+ *
+ * When dealing with translatable entities, the "load_latest_revision" flag will
+ * make this converter load the latest revision affecting the translation
+ * matching the content language for the current request. If none can be found
+ * it will fall back to the latest revision. For instance, if an entity has an
+ * English default revision (revision 1) and an Italian pending revision
+ * (revision 2), "/foo/1" will return the former, while "/it/foo/1" will return
+ * the latter.
+ *
+ * @see entities_revisions_translations
  */
 class EntityConverter implements ParamConverterInterface {
 
@@ -46,14 +72,24 @@ class EntityConverter implements ParamConverterInterface {
    */
   protected $entityManager;
 
+  /**
+   * The language manager.
+   *
+   * @var \Drupal\Core\Language\LanguageManagerInterface
+   */
+  protected $languageManager;
+
   /**
    * Constructs a new EntityConverter.
    *
    * @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager
    *   The entity manager.
+   * @param \Drupal\Core\Language\LanguageManagerInterface|null $language_manager
+   *   (optional) The language manager. Defaults to none.
    */
-  public function __construct(EntityManagerInterface $entity_manager) {
+  public function __construct(EntityManagerInterface $entity_manager, LanguageManagerInterface $language_manager = NULL) {
     $this->entityManager = $entity_manager;
+    $this->languageManager = $language_manager;
   }
 
   /**
@@ -68,14 +104,12 @@ public function convert($value, $definition, $name, array $defaults) {
 
     // If the entity type is revisionable and the parameter has the
     // "load_latest_revision" flag, load the latest revision.
-    if ($entity instanceof ContentEntityInterface && !empty($definition['load_latest_revision']) && $entity_definition->isRevisionable()) {
-      /** @var \Drupal\Core\Entity\ContentEntityStorageInterface  $storage */
-      $latest_revision_id = $storage->getLatestRevisionId($value);
-      // We explicitly perform a loose equality check, since a revision ID may
-      // be returned as an integer or a string.
-      if ($entity->getLoadedRevisionId() != $latest_revision_id) {
-        $entity = $storage->loadRevision($latest_revision_id);
-      }
+    if ($entity instanceof RevisionableInterface && !empty($definition['load_latest_revision']) && $entity_definition->isRevisionable()) {
+      // Retrieve the latest revision ID taking translations into account.
+      $langcode = $this->languageManager()
+        ->getCurrentLanguage(LanguageInterface::TYPE_CONTENT)
+        ->getId();
+      $entity = $this->getLatestTranslationAffectedRevision($entity, $langcode);
     }
 
     // If the entity type is translatable, ensure we return the proper
@@ -87,6 +121,78 @@ public function convert($value, $definition, $name, array $defaults) {
     return $entity;
   }
 
+  /**
+   * Returns the ID of the latest revision translation of the specified entity.
+   *
+   * @param \Drupal\Core\Entity\EntityInterface|\Drupal\Core\Entity\RevisionableInterface $entity
+   *   The default revision of the entity being converted.
+   * @param string $langcode
+   *   The language of the revision translation to be loaded.
+   *
+   * @return \Drupal\Core\Entity\EntityInterface|\Drupal\Core\Entity\RevisionableInterface
+   *   The latest translation-affecting revision for the specified entity, or
+   *   just the latest revision, if the specified entity is not translatable or
+   *   does not have a matching translation yet.
+   */
+  protected function getLatestTranslationAffectedRevision(RevisionableInterface $entity, $langcode) {
+    $revision = NULL;
+    $storage = $this->entityManager->getStorage($entity->getEntityTypeId());
+
+    if ($entity instanceof TranslatableRevisionableInterface && $entity->isTranslatable()) {
+      /** @var \Drupal\Core\Entity\TranslatableRevisionableStorageInterface $storage */
+      $revision_id = $storage->getLatestTranslationAffectedRevisionId($entity->id(), $langcode);
+
+      // If the latest translation-affecting revision was a default revision, it
+      // is fine to load the latest revision instead, because in this case the
+      // latest revision, regardless of it being default or pending, will always
+      // contain the most up-to-date values for the specified translation. This
+      // provides a BC behavior when the route is defined by a module always
+      // expecting the latest revision to be loaded and to be the default
+      // revision. In this particular case the latest revision is always going
+      // to be the default revision, since pending revisions would not be
+      // supported.
+      /** @var \Drupal\Core\Entity\TranslatableRevisionableInterface $revision */
+      $revision = $revision_id ? $this->loadRevision($entity, $revision_id) : NULL;
+      if (!$revision || ($revision->wasDefaultRevision() && !$revision->isDefaultRevision())) {
+        $revision = NULL;
+      }
+    }
+
+    // Fall back to the latest revisions if no affected revision for the current
+    // content language could be found. This is acceptable as it means the
+    // entity is not translated. This is the correct logic also on monolingual
+    // sites.
+    if (!isset($revision)) {
+      $revision_id = $storage->getLatestRevisionId($entity->id());
+      $revision = $this->loadRevision($entity, $revision_id);
+    }
+
+    return $revision;
+  }
+
+  /**
+   * Loads the specified entity revision.
+   *
+   * @param \Drupal\Core\Entity\EntityInterface|\Drupal\Core\Entity\RevisionableInterface $entity
+   *   The default revision of the entity being converted.
+   * @param string $revision_id
+   *   The identifier of the revision to be loaded.
+   *
+   * @return \Drupal\Core\Entity\EntityInterface|\Drupal\Core\Entity\RevisionableInterface
+   *   An entity revision object.
+   */
+  protected function loadRevision(RevisionableInterface $entity, $revision_id) {
+    // We explicitly perform a loose equality check, since a revision ID may
+    // be returned as an integer or a string.
+    if ($entity->getLoadedRevisionId() != $revision_id) {
+      $storage = $this->entityManager->getStorage($entity->getEntityTypeId());
+      return $storage->loadRevision($revision_id);
+    }
+    else {
+      return $entity;
+    }
+  }
+
   /**
    * {@inheritdoc}
    */
@@ -132,4 +238,22 @@ protected function getEntityTypeFromDefaults($definition, $name, array $defaults
     return $entity_type_id;
   }
 
+  /**
+   * Returns a language manager instance.
+   *
+   * @return \Drupal\Core\Language\LanguageManagerInterface
+   *   The language manager.
+   *
+   * @internal
+   */
+  protected function languageManager() {
+    if (!isset($this->languageManager)) {
+      $this->languageManager = \Drupal::languageManager();
+      // @todo Turn this into a proper error (E_USER_ERROR) in
+      //   https://www.drupal.org/node/2938929.
+      @trigger_error('The language manager parameter has been added to EntityConverter since version 8.5.0 and will be made required in version 9.0.0 when requesting the latest translation-affected revision of an entity.', E_USER_DEPRECATED);
+    }
+    return $this->languageManager;
+  }
+
 }

core/modules/content_moderation/src/ParamConverter/EntityRevisionConverter.php

@@ -30,7 +30,7 @@ class EntityRevisionConverter extends EntityConverter {
    *   The moderation info utility service.
    */
   public function __construct(EntityManagerInterface $entity_manager, ModerationInformationInterface $moderation_info) {
-    parent::__construct($entity_manager);
+    parent::__construct($entity_manager, \Drupal::languageManager());
     $this->moderationInformation = $moderation_info;
   }
 

core/modules/content_translation/src/Routing/ContentTranslationRouteSubscriber.php

@@ -70,6 +70,7 @@ protected function alterRoutes(RouteCollection $collection) {
           'parameters' => [
             $entity_type_id => [
               'type' => 'entity:' . $entity_type_id,
+              'load_latest_revision' => TRUE,
             ],
           ],
           '_admin_route' => $is_admin,
@@ -102,6 +103,7 @@ protected function alterRoutes(RouteCollection $collection) {
             ],
             $entity_type_id => [
               'type' => 'entity:' . $entity_type_id,
+              'load_latest_revision' => TRUE,
             ],
           ],
           '_admin_route' => $is_admin,
@@ -127,6 +129,7 @@ protected function alterRoutes(RouteCollection $collection) {
             ],
             $entity_type_id => [
               'type' => 'entity:' . $entity_type_id,
+              'load_latest_revision' => TRUE,
             ],
           ],
           '_admin_route' => $is_admin,
@@ -152,6 +155,7 @@ protected function alterRoutes(RouteCollection $collection) {
             ],
             $entity_type_id => [
               'type' => 'entity:' . $entity_type_id,
+              'load_latest_revision' => TRUE,
             ],
           ],
           '_admin_route' => $is_admin,

core/tests/Drupal/KernelTests/Core/ParamConverter/EntityConverterLatestRevisionTest.php

@@ -96,17 +96,24 @@ public function testEntityWithPendingRevision() {
    * Tests with a translated pending revision.
    */
   public function testWithTranslatedPendingRevision() {
+    // Enable translation for test entities.
+    $this->container->get('state')->set('entity_test.translation', TRUE);
+    $this->container->get('entity_type.bundle.info')->clearCachedBundles();
+
+    // Create a new English entity.
     $entity = EntityTestMulRev::create();
     $entity->save();
 
     // Create a translated pending revision.
-    $translated_entity = $entity->addTranslation('de');
-    $translated_entity->isDefaultRevision(FALSE);
-    $translated_entity->setNewRevision(TRUE);
+    $entity_type_id = 'entity_test_mulrev';
+    /** @var \Drupal\Core\Entity\ContentEntityStorageInterface $storage */
+    $storage = $this->container->get('entity_type.manager')->getStorage($entity_type_id);
+    /** @var \Drupal\Core\Entity\ContentEntityInterface $translated_entity */
+    $translated_entity = $storage->createRevision($entity->addTranslation('de'), FALSE);
     $translated_entity->save();
 
     // Change the site language so the converters will attempt to load entities
-    // with 'de'.
+    // with language 'de'.
     $this->config('system.site')->set('default_langcode', 'de')->save();
 
     // The default loaded language is still 'en'.
@@ -120,6 +127,17 @@ public function testWithTranslatedPendingRevision() {
     ], 'foo', []);
     $this->assertEquals('de', $converted->language()->getId());
     $this->assertEquals($translated_entity->getLoadedRevisionId(), $converted->getLoadedRevisionId());
+
+    // Revert back to English as default language.
+    $this->config('system.site')->set('default_langcode', 'en')->save();
+
+    // The converter will load the latest revision in the correct language.
+    $converted = $this->converter->convert(1, [
+      'load_latest_revision' => TRUE,
+      'type' => 'entity:entity_test_mulrev',
+    ], 'foo', []);
+    $this->assertEquals('en', $converted->language()->getId());
+    $this->assertEquals($entity->getLoadedRevisionId(), $converted->getLoadedRevisionId());
   }
 
   /**

core/tests/Drupal/Tests/Core/ParamConverter/EntityConverterTest.php

@@ -3,9 +3,15 @@
 namespace Drupal\Tests\Core\ParamConverter;
 
 use Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException;
+use Drupal\Core\Entity\ContentEntityInterface;
+use Drupal\Core\Entity\ContentEntityStorageInterface;
+use Drupal\Core\Entity\ContentEntityTypeInterface;
+use Drupal\Core\Language\LanguageInterface;
+use Drupal\Core\Language\LanguageManagerInterface;
 use Drupal\Core\ParamConverter\EntityConverter;
 use Drupal\Core\ParamConverter\ParamNotConvertedException;
 use Drupal\Tests\UnitTestCase;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\Routing\Route;
 
 /**
@@ -130,4 +136,74 @@ public function testConvertWithInvalidDynamicEntityType() {
     $this->entityConverter->convert('id', ['type' => 'entity:{invalid_id}'], 'foo', ['foo' => 'id']);
   }
 
+  /**
+   * Tests that omitting the language manager triggers a deprecation error.
+   *
+   * @group legacy
+   *
+   * @expectedDeprecation The language manager parameter has been added to EntityConverter since version 8.5.0 and will be made required in version 9.0.0 when requesting the latest translation-affected revision of an entity.
+   */
+  public function testDeprecatedOptionalLanguageManager() {
+    $entity = $this->createMock(ContentEntityInterface::class);
+    $entity->expects($this->any())
+      ->method('getEntityTypeId')
+      ->willReturn('entity_test');
+    $entity->expects($this->any())
+      ->method('id')
+      ->willReturn('id');
+    $entity->expects($this->any())
+      ->method('isTranslatable')
+      ->willReturn(FALSE);
+    $entity->expects($this->any())
+      ->method('getLoadedRevisionId')
+      ->willReturn('revision_id');
+
+    $storage = $this->createMock(ContentEntityStorageInterface::class);
+    $storage->expects($this->any())
+      ->method('load')
+      ->with('id')
+      ->willReturn($entity);
+    $storage->expects($this->any())
+      ->method('getLatestRevisionId')
+      ->with('id')
+      ->willReturn('revision_id');
+
+    $this->entityManager->expects($this->any())
+      ->method('getStorage')
+      ->with('entity_test')
+      ->willReturn($storage);
+
+    $entity_type = $this->createMock(ContentEntityTypeInterface::class);
+    $entity_type->expects($this->any())
+      ->method('isRevisionable')
+      ->willReturn(TRUE);
+
+    $this->entityManager->expects($this->any())
+      ->method('getDefinition')
+      ->with('entity_test')
+      ->willReturn($entity_type);
+
+    $language = $this->createMock(LanguageInterface::class);
+    $language->expects($this->any())
+      ->method('getId')
+      ->willReturn('en');
+
+    $language_manager = $this->createMock(LanguageManagerInterface::class);
+    $language_manager->expects($this->any())
+      ->method('getCurrentLanguage')
+      ->with(LanguageInterface::TYPE_CONTENT)
+      ->willReturn($language);
+
+    /** @var \Symfony\Component\DependencyInjection\ContainerInterface|\PHPUnit_Framework_MockObject_MockObject $container */
+    $container = $this->createMock(ContainerInterface::class);
+    $container->expects($this->any())
+      ->method('get')
+      ->with('language_manager')
+      ->willReturn($language_manager);
+
+    \Drupal::setContainer($container);
+    $definition = ['type' => 'entity:entity_test', 'load_latest_revision' => TRUE];
+    $this->entityConverter->convert('id', $definition, 'foo', []);
+  }
+
 }