DRAFT: Random cleanup

modules/common/src/Storage/AbstractDatabaseTable.php

@@ -305,6 +305,8 @@ protected function tableExist($table_name): bool {
    */
   protected function tableCreate($table_name, $schema) {
     // Opportunity to further alter the schema before table creation.
+    // @todo Give this event its own event class. Pass the schema array by
+    //   reference.
     $schema = $this->dispatchEvent(self::EVENT_TABLE_CREATE, $schema);
 
     $this->connection->schema()->createTable($table_name, $schema);

modules/datastore/src/Service/ImportService.php

@@ -18,9 +18,10 @@
 /**
  * Datastore importer.
  *
- * @todo This class has state and is not actually a service because it holds
- *   state. Have import() take an argument of a resource, instead of storing it
- *   as a property.
+ * Use the factory service (dkan.datastore.service.factory.import) to generate
+ * these.
+ *
+ * @see \Drupal\datastore\Service\Factory\ImportServiceFactory::getInstance
  */
 class ImportService {
 
@@ -144,7 +145,7 @@ protected function getResource(): DataResource {
   }
 
   /**
-   * Import.
+   * Import resources into storage.
    */
   public function import() {
     $result = $this->getImporter()->run();

modules/metastore/metastore.module

@@ -12,33 +12,34 @@ use Drupal\metastore\MetastoreService;
 use Drupal\metastore\Storage\MetastoreEntityStorageInterface;
 
 /**
- * Implements hook_entity_load().
+ * Implements hook_ENTITY_TYPE_load().
+ *
+ * @see Drupal\metastore\LifeCycle\LifeCycle::datasetLoad()
+ * @see Drupal\metastore\LifeCycle\LifeCycle::distributionLoad()
  */
-function metastore_entity_load(array $entities) {
-  foreach($entities as $entity) {
-    metastore_data_lifecycle($entity, "load");
-  }
+function metastore_node_load(array $entities) {
+  metastore_data_lifecycle($entities, 'load');
 }
 
 /**
- * Implements hook_entity_presave().
+ * Implements hook_ENTITY_TYPE_presave().
  */
-function metastore_entity_presave(EntityInterface $entity) {
-  metastore_data_lifecycle($entity, "presave");
+function metastore_node_presave(EntityInterface $entity) {
+  metastore_data_lifecycle([$entity], 'presave');
 }
 
 /**
- * Implements hook_entity_predelete().
+ * Implements hook_ENTITY_TYPE_predelete().
  */
-function metastore_entity_predelete(EntityInterface $entity) {
-  metastore_data_lifecycle($entity, "predelete");
+function metastore_node_predelete(EntityInterface $entity) {
+  metastore_data_lifecycle([$entity], 'predelete');
 }
 
 /**
- * Implements hook_entity_update().
+ * Implements hook_ENTITY_TYPE_update().
  */
-function metastore_entity_update(EntityInterface $entity) {
-  metastore_data_lifecycle($entity, "update");
+function metastore_node_update(EntityInterface $entity) {
+  metastore_data_lifecycle([$entity], 'update');
 }
 
 /**
@@ -60,22 +61,24 @@ function resource_mapper_new_revision() {
 }
 
 /**
- * Create a LifeCycle object.
- *
- * @param Drupal\Core\Entity\ContentEntityInterface $entity
+ * Send the hook off to the lifecycle service.
  *
- * @return Drupal\metastore\LifeCycle\Data
- *   LifeCycle object.
+ * @param Drupal\Core\Entity\ContentEntityInterface[] $entities
+ *   The entities in question.
+ * @param string $stage
+ *   The entity hook stage, such as 'load' or 'update'.
  */
-function metastore_data_lifecycle(EntityInterface $entity, string $stage) {
-
-  if (metastore_entity_is_valid_item($entity)) {
-    $itemFactory = \Drupal::service('dkan.metastore.metastore_item_factory');
-    $metastoreItem = $itemFactory->wrap($entity);
-    \Drupal::service('dkan.metastore.lifecycle')->go($stage, $metastoreItem);
+function metastore_data_lifecycle(array $entities, string $stage): void {
+  /** @var \Drupal\metastore\LifeCycle\LifeCycle $life_cycle */
+  $life_cycle = \Drupal::service('dkan.metastore.lifecycle');
+  /** @var \Drupal\metastore\Factory\MetastoreEntityItemFactoryInterface $itemFactory */
+  $itemFactory = \Drupal::service('dkan.metastore.metastore_item_factory');
+  foreach ($entities as $entity) {
+    if ($life_cycle->entityIsValidItem($entity)) {
+      $metastoreItem = $itemFactory->wrap($entity);
+      $life_cycle->go($stage, $metastoreItem);
+    }
   }
-
-  return FALSE;
 }
 
 /**
@@ -86,25 +89,13 @@ function metastore_data_lifecycle(EntityInterface $entity, string $stage) {
  *
  * @return bool
  *   Returns true if the entity is used by the metastore.
+ *
+ * @deprecated Use LifeCycle::entityIsValidItem().
  */
 function metastore_entity_is_valid_item(EntityInterface $entity) {
-  $storageClass = \Drupal::service('dkan.metastore.storage')::getStorageClass();
-
-  // If the storage class used implements the entity storage interface, continue.
-  if (!is_a($storageClass, MetastoreEntityStorageInterface::class, true)) {
-    return FALSE;
-  }
-
-  $storageEntityType = \Drupal::service('dkan.metastore.metastore_item_factory')::getEntityType();
-  $storageBundles = \Drupal::service('dkan.metastore.metastore_item_factory')::getBundles();
-
-  // If the type and bundle are correct, return true.
-  if ($entity->getEntityTypeId() != $storageEntityType) {
-    return FALSE;
-  }
-  if (in_array($entity->bundle(), $storageBundles)) {
-    return TRUE;
-  }
+  /** @var \Drupal\metastore\LifeCycle\LifeCycle $life_cycle */
+  $life_cycle = \Drupal::service('dkan.metastore.lifecycle');
+  return $life_cycle->entityIsValidItem($entity);
 }
 
 /**

modules/metastore/src/Factory/MetastoreItemFactoryInterface.php

@@ -5,13 +5,14 @@
 use Contracts\FactoryInterface;
 use Drupal\Core\Entity\EntityRepository;
 use Drupal\Core\Entity\EntityTypeManager;
+use Drupal\metastore\MetastoreItemInterface;
 
 /**
  * Interface MetastoreItemFactoryInterface.
  *
- * Used for service dkan.metastore.metastore_item_factory. Override the service
+ * Used for service dkan.metastore.metastore_item_factory. Decorate the service
  * to use different logic for producing a MetastoreItemInterface object from
- * just an indentifier.
+ * just an identifier.
  */
 interface MetastoreItemFactoryInterface extends FactoryInterface {
 
@@ -36,18 +37,18 @@ public function __construct(EntityRepository $entityRepository, EntityTypeManage
    * @return \Drupal\metastore\MetastoreItemInterface
    *   A metastore item object.
    */
-  public function getInstance(string $identifier, array $config = []);
+  public function getInstance(string $identifier, array $config = []): MetastoreItemInterface;
 
   /**
    * Wrap an arbitrary object as a metastore item interface compliant object.
    *
-   * @param mixed $input
+   * @param object $input
    *   Any object that can be wrapped as a metastore item. For instance, a node.
    *
-   * @return Drupal\metastore\MetastoreItemInterface
-   *   A metastore item interface compliant object.
+   * @return \Drupal\metastore\MetastoreItemInterface
+   *   A wrapper that implements MetastoreItemInterface.
    */
-  public function wrap(mixed $input);
+  public function wrap(object $input): MetastoreItemInterface;
 
   /**
    * Return list cache tags for metastore items.

modules/metastore/src/LifeCycle/LifeCycle.php

@@ -2,23 +2,25 @@
 
 namespace Drupal\metastore\LifeCycle;
 
-use Drupal\common\EventDispatcherTrait;
-use Drupal\common\DataResource;
-use Drupal\common\Exception\DataNodeLifeCycleEntityValidationException;
-use Drupal\common\UrlHostTokenResolver;
 use Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException;
 use Drupal\Component\Plugin\Exception\PluginNotFoundException;
 use Drupal\Core\Config\ConfigFactory;
 use Drupal\Core\Datetime\DateFormatter;
+use Drupal\Core\Entity\ContentEntityInterface;
 use Drupal\Core\Queue\QueueFactory;
 use Drupal\Core\StreamWrapper\StreamWrapperManager;
+use Drupal\common\DataResource;
+use Drupal\common\EventDispatcherTrait;
+use Drupal\common\Exception\DataNodeLifeCycleEntityValidationException;
+use Drupal\common\UrlHostTokenResolver;
 use Drupal\metastore\MetastoreItemInterface;
 use Drupal\metastore\Reference\Dereferencer;
 use Drupal\metastore\Reference\MetastoreUrlGenerator;
 use Drupal\metastore\Reference\OrphanChecker;
 use Drupal\metastore\Reference\Referencer;
 use Drupal\metastore\ResourceMapper;
 use Drupal\metastore\Storage\DataFactory;
+use Drupal\metastore\Storage\MetastoreEntityStorageInterface;
 
 /**
  * Abstraction of logic used in entity hooks.
@@ -103,7 +105,7 @@ public function __construct(
     DateFormatter $dateFormatter,
     DataFactory $dataFactory,
     QueueFactory $queueFactory,
-    ConfigFactory $configFactory
+    ConfigFactory $configFactory,
   ) {
     $this->referencer = $referencer;
     $this->dereferencer = $dereferencer;
@@ -115,9 +117,51 @@ public function __construct(
     $this->configFactory = $configFactory;
   }
 
+  /**
+   * Check if the entity is part of the metastore.
+   *
+   * @param \Drupal\Core\Entity\ContentEntityInterface $entity
+   *   A Drupal content entity.
+   *
+   * @return bool
+   *   Returns true if the entity is used by the metastore.
+   */
+  public function entityIsValidItem(ContentEntityInterface $entity) {
+    // @todo Inject this.
+    $storageClass = \Drupal::service('dkan.metastore.storage')::getStorageClass();
+
+    // If the storage class used implements the entity storage interface,
+    // continue.
+    // @todo Should we look at the entity's storage class instead?
+    if (!is_a($storageClass, MetastoreEntityStorageInterface::class, TRUE)) {
+      return FALSE;
+    }
+
+    // @todo Inject these.
+    $storageEntityType = \Drupal::service('dkan.metastore.metastore_item_factory')::getEntityType();
+    $storageBundles = \Drupal::service('dkan.metastore.metastore_item_factory')::getBundles();
+
+    // If the type and bundle are correct, return true.
+    if ($entity->getEntityTypeId() != $storageEntityType) {
+      return FALSE;
+    }
+    if (in_array($entity->bundle(), $storageBundles)) {
+      return TRUE;
+    }
+    return FALSE;
+  }
+
   /**
    * Entry point for LifeCycle functions.
    *
+   * Based on the schema of the $data object and the $stage, we generate a
+   * method name. If that method name exists on this class, we call it.
+   * Example: Stage 'load' for a dataset metastore item becomes 'datasetLoad'.
+   *
+   * Currently, this method handles hook implementations for Data nodes via
+   * wrappers, but might be expected to handle arbitrary entities in the
+   * future.
+   *
    * @param string $stage
    *   Stage or hook name for execution.
    * @param \Drupal\metastore\MetastoreItemInterface $data
@@ -126,13 +170,12 @@ public function __construct(
   public function go(string $stage, MetastoreItemInterface $data): void {
     // Removed dashes from schema ID since function names can't include dashes.
     $schema_id = str_replace('-', '', $data->getSchemaId());
-    $stage = ucwords($stage);
     // Build method name from schema ID and stage.
-    $method = "{$schema_id}{$stage}";
+    $method = $schema_id . ucwords($stage);
     // Ensure a method exists for this life cycle stage.
     if (method_exists($this, $method)) {
       // Call life cycle method on metastore item.
-      $this->{$method}($data);
+      $this->$method($data);
     }
   }
 
@@ -149,6 +192,12 @@ protected function datasetPredelete(MetastoreItemInterface $data) {
 
   /**
    * Dataset load.
+   *
+   * @todo This behavior should be on-demand instead of always happening when
+   *   the node loads, since not all dataset nodes will need to be
+   *   dereferenced.
+   *
+   * @see \metastore_node_load()
    */
   protected function datasetLoad(MetastoreItemInterface $data) {
     $metadata = $data->getMetaData();
@@ -179,6 +228,12 @@ protected function datasetUpdate(MetastoreItemInterface $data) {
    * @todo For consistency, this should either be abstracted so that it is not
    * so tightly coupled with the distribution schema, or we should better
    * document that DKAN only supports DCAT standard.
+   *
+   * @todo This behavior should be on-demand instead of always happening when
+   *   the node loads, since not all node loads will need dereferenced download
+   *   URLs.
+   *
+   * @see \metastore_node_load()
    */
   protected function distributionLoad(MetastoreItemInterface $data) {
     $metadata = $data->getMetaData();

modules/metastore/src/MetastoreItemInterface.php

@@ -25,7 +25,10 @@ public function getIdentifier();
   public function getRawMetadata();
 
   /**
-   * Protected.
+   * Get the node schema identifier.
+   *
+   * @return string
+   *   The Data node schema identifier, such as 'dataset' or 'distribution'.
    */
   public function getSchemaId();