Issue #2900421 by timmillwood, tim.plunkett, catch, Sam152: Architectural...

Issue #2900421 by timmillwood, tim.plunkett, catch, Sam152: Architectural review of the Content Moderation module

core/modules/content_moderation/content_moderation.module

@@ -129,9 +129,6 @@ function content_moderation_form_alter(&$form, FormStateInterface $form_state, $
 
 /**
  * Implements hook_preprocess_HOOK().
- *
- * Many default node templates rely on $page to determine whether to output the
- * node title as part of the node content.
  */
 function content_moderation_preprocess_node(&$variables) {
   \Drupal::service('class_resolver')
@@ -287,7 +284,11 @@ function content_moderation_workflow_insert(WorkflowInterface $entity) {
  * Implements hook_ENTITY_TYPE_update().
  */
 function content_moderation_workflow_update(WorkflowInterface $entity) {
-  content_moderation_workflow_insert($entity);
+  // Clear bundle cache so workflow gets added or removed from the bundle
+  // information.
+  \Drupal::service('entity_type.bundle.info')->clearCachedBundles();
+  // Clear field cache so extra field is added or removed.
+  \Drupal::service('entity_field.manager')->clearCachedFieldDefinitions();
 }
 
 /**

core/modules/content_moderation/src/Access/LatestRevisionCheck.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\content_moderation\Access;
 
+use Drupal\Core\Access\AccessException;
 use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Routing\Access\AccessInterface;
@@ -81,10 +82,8 @@ public function access(Route $route, RouteMatchInterface $route_match, AccountIn
    * @return \Drupal\Core\Entity\ContentEntityInterface
    *   returns the Entity in question.
    *
-   * @throws \Exception
-   *   A generic exception is thrown if the entity couldn't be loaded. This
-   *   almost always implies a developer error, so it should get turned into
-   *   an HTTP 500.
+   * @throws \Drupal\Core\Access\AccessException
+   *   An AccessException is thrown if the entity couldn't be loaded.
    */
   protected function loadEntity(Route $route, RouteMatchInterface $route_match) {
     $entity_type = $route->getOption('_content_moderation_entity_type');
@@ -94,7 +93,7 @@ protected function loadEntity(Route $route, RouteMatchInterface $route_match) {
         return $entity;
       }
     }
-    throw new \Exception(sprintf('%s is not a valid entity route. The LatestRevisionCheck access checker may only be used with a route that has a single entity parameter.', $route_match->getRouteName()));
+    throw new AccessException(sprintf('%s is not a valid entity route. The LatestRevisionCheck access checker may only be used with a route that has a single entity parameter.', $route_match->getRouteName()));
   }
 
 }

core/modules/content_moderation/src/ContentPreprocess.php

@@ -41,10 +41,10 @@ public static function create(ContainerInterface $container) {
   }
 
   /**
-   * Wrapper for hook_preprocess_HOOK().
-   *
    * @param array $variables
    *   Theme variables to preprocess.
+   *
+   * @see hook_preprocess_HOOK()
    */
   public function preprocessNode(array &$variables) {
     // Set the 'page' template variable when the node is being displayed on the

core/modules/content_moderation/src/Entity/ContentModerationState.php

@@ -2,7 +2,6 @@
 
 namespace Drupal\content_moderation\Entity;
 
-use Drupal\content_moderation\ContentModerationStateInterface;
 use Drupal\Core\Entity\ContentEntityBase;
 use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Entity\EntityTypeInterface;
@@ -148,7 +147,7 @@ public static function updateOrCreateFromEntity(ContentModerationState $content_
    * @param \Drupal\Core\Entity\EntityInterface $entity
    *   A moderated entity object.
    *
-   * @return \Drupal\content_moderation\ContentModerationStateInterface|null
+   * @return \Drupal\content_moderation\Entity\ContentModerationStateInterface|null
    *   The related content moderation state or NULL if none could be found.
    *
    * @internal
@@ -172,7 +171,7 @@ public static function loadFromModeratedEntity(EntityInterface $entity) {
         ->execute();
 
       if ($ids) {
-        /** @var \Drupal\content_moderation\ContentModerationStateInterface $content_moderation_state */
+        /** @var \Drupal\content_moderation\Entity\ContentModerationStateInterface $content_moderation_state */
         $content_moderation_state = $storage->loadRevision(key($ids));
       }
     }

core/modules/content_moderation/src/ContentModerationStateInterface.php → core/modules/content_moderation/src/Entity/ContentModerationStateInterface.php

 <?php
 
-namespace Drupal\content_moderation;
+namespace Drupal\content_moderation\Entity;
 
 use Drupal\Core\Entity\ContentEntityInterface;
 use Drupal\user\EntityOwnerInterface;

core/modules/content_moderation/src/Entity/Handler/BlockContentModerationHandler.php

@@ -6,6 +6,8 @@
 
 /**
  * Customizations for block content entities.
+ *
+ * @internal
  */
 class BlockContentModerationHandler extends ModerationHandler {
 

core/modules/content_moderation/src/Entity/Handler/ModerationHandler.php

@@ -14,6 +14,8 @@
  * Common customizations for most/all entities.
  *
  * This class is intended primarily as a base class.
+ *
+ * @internal
  */
 class ModerationHandler implements ModerationHandlerInterface, EntityHandlerInterface {
 

core/modules/content_moderation/src/Entity/Handler/ModerationHandlerInterface.php

@@ -11,6 +11,8 @@
  * Much of the logic contained in this handler is an indication of flaws
  * in the Entity API that are insufficiently standardized between entity types.
  * Hopefully over time functionality can be removed from this interface.
+ *
+ * @internal
  */
 interface ModerationHandlerInterface {
 

core/modules/content_moderation/src/Entity/Handler/NodeModerationHandler.php

@@ -9,6 +9,8 @@
 
 /**
  * Customizations for node entities.
+ *
+ * @internal
  */
 class NodeModerationHandler extends ModerationHandler {
 

core/modules/content_moderation/src/Routing/EntityModerationRouteProvider.php → core/modules/content_moderation/src/Entity/Routing/EntityModerationRouteProvider.php

 <?php
 
-namespace Drupal\content_moderation\Routing;
+namespace Drupal\content_moderation\Entity\Routing;
 
 use Drupal\Core\Entity\EntityFieldManagerInterface;
 use Drupal\Core\Entity\EntityHandlerInterface;

core/modules/content_moderation/src/EntityOperations.php

@@ -3,6 +3,7 @@
 namespace Drupal\content_moderation;
 
 use Drupal\content_moderation\Entity\ContentModerationState as ContentModerationStateEntity;
+use Drupal\content_moderation\Entity\ContentModerationStateInterface;
 use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
 use Drupal\Core\Entity\Display\EntityViewDisplayInterface;
 use Drupal\Core\Entity\EntityInterface;
@@ -94,6 +95,8 @@ public static function create(ContainerInterface $container) {
    *
    * @param \Drupal\Core\Entity\EntityInterface $entity
    *   The entity being saved.
+   *
+   * @see hook_entity_presave()
    */
   public function entityPresave(EntityInterface $entity) {
     if (!$this->moderationInfo->isModeratedEntity($entity)) {
@@ -103,7 +106,8 @@ public function entityPresave(EntityInterface $entity) {
     if ($entity->moderation_state->value) {
       $workflow = $this->moderationInfo->getWorkflowForEntity($entity);
       /** @var \Drupal\content_moderation\ContentModerationState $current_state */
-      $current_state = $workflow->getTypePlugin()->getState($entity->moderation_state->value);
+      $current_state = $workflow->getTypePlugin()
+        ->getState($entity->moderation_state->value);
 
       // This entity is default if it is new, a new translation, the default
       // revision, or the default revision is not published.
@@ -113,13 +117,13 @@ public function entityPresave(EntityInterface $entity) {
         || !$this->moderationInfo->isDefaultRevisionPublished($entity);
 
       // Fire per-entity-type logic for handling the save process.
-      $this->entityTypeManager->getHandler($entity->getEntityTypeId(), 'moderation')->onPresave($entity, $update_default_revision, $current_state->isPublishedState());
+      $this->entityTypeManager
+        ->getHandler($entity->getEntityTypeId(), 'moderation')
+        ->onPresave($entity, $update_default_revision, $current_state->isPublishedState());
     }
   }
 
   /**
-   * Hook bridge.
-   *
    * @param \Drupal\Core\Entity\EntityInterface $entity
    *   The entity that was just saved.
    *
@@ -133,8 +137,6 @@ public function entityInsert(EntityInterface $entity) {
   }
 
   /**
-   * Hook bridge.
-   *
    * @param \Drupal\Core\Entity\EntityInterface $entity
    *   The entity that was just saved.
    *
@@ -217,8 +219,6 @@ protected function setLatestRevision(EntityInterface $entity) {
   }
 
   /**
-   * Hook bridge.
-   *
    * @param \Drupal\Core\Entity\EntityInterface $entity
    *   The entity being deleted.
    *
@@ -232,8 +232,6 @@ public function entityDelete(EntityInterface $entity) {
   }
 
   /**
-   * Hook bridge.
-   *
    * @param \Drupal\Core\Entity\EntityInterface $entity
    *   The entity revision being deleted.
    *
@@ -252,8 +250,6 @@ public function entityRevisionDelete(EntityInterface $entity) {
   }
 
   /**
-   * Hook bridge.
-   *
    * @param \Drupal\Core\Entity\EntityInterface $translation
    *   The entity translation being deleted.
    *
@@ -274,8 +270,6 @@ public function entityTranslationDelete(EntityInterface $translation) {
   /**
    * Act on entities being assembled before rendering.
    *
-   * This is a hook bridge.
-   *
    * @see hook_entity_view()
    * @see EntityFieldManagerInterface::getExtraFields()
    */

core/modules/content_moderation/src/EntityTypeInfo.php

@@ -18,7 +18,7 @@
 use Drupal\content_moderation\Entity\Handler\BlockContentModerationHandler;
 use Drupal\content_moderation\Entity\Handler\ModerationHandler;
 use Drupal\content_moderation\Entity\Handler\NodeModerationHandler;
-use Drupal\content_moderation\Routing\EntityModerationRouteProvider;
+use Drupal\content_moderation\Entity\Routing\EntityModerationRouteProvider;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
@@ -121,8 +121,6 @@ public static function create(ContainerInterface $container) {
   /**
    * Adds Moderation configuration to appropriate entity types.
    *
-   * This is an alter hook bridge.
-   *
    * @param EntityTypeInterface[] $entity_types
    *   The master entity type list to alter.
    *
@@ -171,10 +169,6 @@ protected function addModerationToEntityType(ContentEntityTypeInterface $type) {
   /**
    * Gets the "extra fields" for a bundle.
    *
-   * This is a hook bridge.
-   *
-   * @see hook_entity_extra_field_info()
-   *
    * @return array
    *   A nested array of 'pseudo-field' elements. Each list is nested within the
    *   following keys: entity type, bundle name, context (either 'form' or
@@ -193,6 +187,8 @@ protected function addModerationToEntityType(ContentEntityTypeInterface $type) {
    *   - delete: (optional) String containing markup (normally a link) used as
    *     the element's 'delete' operation in the administration interface. Only
    *     for 'form' context.
+   *
+   * @see hook_entity_extra_field_info()
    */
   public function entityExtraFieldInfo() {
     $return = [];
@@ -239,6 +235,8 @@ protected function getModeratedBundles() {
    *
    * @return \Drupal\Core\Field\BaseFieldDefinition[]
    *   New fields added by moderation state.
+   *
+   * @see hook_entity_base_field_info()
    */
   public function entityBaseFieldInfo(EntityTypeInterface $entity_type) {
     if (!$this->moderationInfo->canModerateEntitiesOfEntityType($entity_type)) {
@@ -251,7 +249,6 @@ public function entityBaseFieldInfo(EntityTypeInterface $entity_type) {
       ->setDescription(t('The moderation state of this piece of content.'))
       ->setComputed(TRUE)
       ->setClass(ModerationStateFieldItemList::class)
-      ->setSetting('target_type', 'moderation_state')
       ->setDisplayOptions('view', [
         'label' => 'hidden',
         'region' => 'hidden',

core/modules/content_moderation/src/Plugin/Field/ModerationStateFieldItemList.php

@@ -48,7 +48,7 @@ protected function getModerationStateId() {
    * @param \Drupal\Core\Entity\ContentEntityInterface $entity
    *   The entity the content moderation state entity will be loaded from.
    *
-   * @return \Drupal\content_moderation\ContentModerationStateInterface|null
+   * @return \Drupal\content_moderation\Entity\ContentModerationStateInterface|null
    *   The content_moderation_state revision or FALSE if none exists.
    */
   protected function loadContentModerationStateRevision(ContentEntityInterface $entity) {
@@ -69,7 +69,7 @@ protected function loadContentModerationStateRevision(ContentEntityInterface $en
       return NULL;
     }
 
-    /** @var \Drupal\content_moderation\ContentModerationStateInterface $content_moderation_state */
+    /** @var \Drupal\content_moderation\Entity\ContentModerationStateInterface $content_moderation_state */
     $content_moderation_state = $content_moderation_storage->loadRevision(key($revisions));
     if ($entity->getEntityType()->hasKey('langcode')) {
       $langcode = $entity->language()->getId();

core/modules/content_moderation/src/Plugin/Validation/Constraint/ModerationStateConstraintValidator.php

@@ -6,7 +6,6 @@
 use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Drupal\content_moderation\ModerationInformationInterface;
-use Drupal\content_moderation\StateTransitionValidation;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\ConstraintValidator;
@@ -16,13 +15,6 @@
  */
 class ModerationStateConstraintValidator extends ConstraintValidator implements ContainerInjectionInterface {
 
-  /**
-   * The state transition validation.
-   *
-   * @var \Drupal\content_moderation\StateTransitionValidation
-   */
-  protected $validation;
-
   /**
    * The entity type manager.
    *
@@ -42,13 +34,10 @@ class ModerationStateConstraintValidator extends ConstraintValidator implements
    *
    * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
    *   The entity type manager.
-   * @param \Drupal\content_moderation\StateTransitionValidation $validation
-   *   The state transition validation.
    * @param \Drupal\content_moderation\ModerationInformationInterface $moderation_information
    *   The moderation information.
    */
-  public function __construct(EntityTypeManagerInterface $entity_type_manager, StateTransitionValidation $validation, ModerationInformationInterface $moderation_information) {
-    $this->validation = $validation;
+  public function __construct(EntityTypeManagerInterface $entity_type_manager, ModerationInformationInterface $moderation_information) {
     $this->entityTypeManager = $entity_type_manager;
     $this->moderationInformation = $moderation_information;
   }
@@ -59,7 +48,6 @@ public function __construct(EntityTypeManagerInterface $entity_type_manager, Sta
   public static function create(ContainerInterface $container) {
     return new static(
       $container->get('entity_type.manager'),
-      $container->get('content_moderation.state_transition_validation'),
       $container->get('content_moderation.moderation_information')
     );
   }

core/modules/content_moderation/src/Plugin/WorkflowType/ContentModeration.php

@@ -31,7 +31,7 @@
  *   },
  * )
  */
-class ContentModeration extends WorkflowTypeBase implements ContainerFactoryPluginInterface {
+class ContentModeration extends WorkflowTypeBase implements ContentModerationInterface, ContainerFactoryPluginInterface {
 
   use StringTranslationTrait;
 
@@ -135,52 +135,28 @@ public function workflowStateHasData(WorkflowInterface $workflow, StateInterface
   }
 
   /**
-   * Gets the entity types the workflow is applied to.
-   *
-   * @return string[]
-   *   The entity types the workflow is applied to.
+   * {@inheritdoc}
    */
   public function getEntityTypes() {
     return array_keys($this->configuration['entity_types']);
   }
 
   /**
-   * Gets any bundles the workflow is applied to for the given entity type.
-   *
-   * @param string $entity_type_id
-   *   The entity type ID to get the bundles for.
-   *
-   * @return string[]
-   *   The bundles of the entity type the workflow is applied to or an empty
-   *   array if the entity type is not applied to the workflow.
+   * {@inheritdoc}
    */
   public function getBundlesForEntityType($entity_type_id) {
     return isset($this->configuration['entity_types'][$entity_type_id]) ? $this->configuration['entity_types'][$entity_type_id] : [];
   }
 
   /**
-   * Checks if the workflow applies to the supplied entity type and bundle.
-   *
-   * @param string $entity_type_id
-   *   The entity type ID to check.
-   * @param string $bundle_id
-   *   The bundle ID to check.
-   *
-   * @return bool
-   *   TRUE if the workflow applies to the supplied entity type ID and bundle
-   *   ID. FALSE if not.
+   * {@inheritdoc}
    */
   public function appliesToEntityTypeAndBundle($entity_type_id, $bundle_id) {
     return in_array($bundle_id, $this->getBundlesForEntityType($entity_type_id), TRUE);
   }
 
   /**
-   * Removes an entity type ID / bundle ID from the workflow.
-   *
-   * @param string $entity_type_id
-   *   The entity type ID to remove.
-   * @param string $bundle_id
-   *   The bundle ID to remove.
+   * {@inheritdoc}
    */
   public function removeEntityTypeAndBundle($entity_type_id, $bundle_id) {
     if (!isset($this->configuration['entity_types'][$entity_type_id])) {
@@ -199,14 +175,7 @@ public function removeEntityTypeAndBundle($entity_type_id, $bundle_id) {
   }
 
   /**
-   * Add an entity type ID / bundle ID to the workflow.
-   *
-   * @param string $entity_type_id
-   *   The entity type ID to add. It is responsibility of the caller to provide
-   *   a valid entity type ID.
-   * @param string $bundle_id
-   *   The bundle ID to add. It is responsibility of the caller to provide a
-   *   valid bundle ID.
+   * {@inheritdoc}
    */
   public function addEntityTypeAndBundle($entity_type_id, $bundle_id) {
     if (!$this->appliesToEntityTypeAndBundle($entity_type_id, $bundle_id)) {

core/modules/content_moderation/src/Plugin/WorkflowType/ContentModerationInterface.php

+<?php
+
+namespace Drupal\content_moderation\Plugin\WorkflowType;
+
+use Drupal\workflows\WorkflowTypeInterface;
+
+/**
+ * Interface for ContentModeration WorkflowType plugin.
+ */
+interface ContentModerationInterface extends WorkflowTypeInterface {
+
+  /**
+   * Gets the entity types the workflow is applied to.
+   *
+   * @return string[]
+   *   The entity types the workflow is applied to.
+   */
+  public function getEntityTypes();
+
+  /**
+   * Gets any bundles the workflow is applied to for the given entity type.
+   *
+   * @param string $entity_type_id
+   *   The entity type ID to get the bundles for.
+   *
+   * @return string[]
+   *   The bundles of the entity type the workflow is applied to or an empty
+   *   array if the entity type is not applied to the workflow.
+   */
+  public function getBundlesForEntityType($entity_type_id);
+
+  /**
+   * Checks if the workflow applies to the supplied entity type and bundle.
+   *
+   * @param string $entity_type_id
+   *   The entity type ID to check.
+   * @param string $bundle_id
+   *   The bundle ID to check.
+   *
+   * @return bool
+   *   TRUE if the workflow applies to the supplied entity type ID and bundle
+   *   ID. FALSE if not.
+   */
+  public function appliesToEntityTypeAndBundle($entity_type_id, $bundle_id);
+
+  /**
+   * Removes an entity type ID / bundle ID from the workflow.
+   *
+   * @param string $entity_type_id
+   *   The entity type ID to remove.
+   * @param string $bundle_id
+   *   The bundle ID to remove.
+   */
+  public function removeEntityTypeAndBundle($entity_type_id, $bundle_id);
+
+  /**
+   * Add an entity type ID / bundle ID to the workflow.
+   *
+   * @param string $entity_type_id
+   *   The entity type ID to add. It is responsibility of the caller to provide
+   *   a valid entity type ID.
+   * @param string $bundle_id
+   *   The bundle ID to add. It is responsibility of the caller to provide a
+   *   valid bundle ID.
+   */
+  public function addEntityTypeAndBundle($entity_type_id, $bundle_id);
+
+  /**
+   * {@inheritdoc}
+   *
+   * @param $entity
+   *   Content Moderation uses this parameter to determine the initial state
+   *   based on publishing status.
+   */
+  public function getInitialState($entity = NULL);
+
+}