diff --git a/modules/common/src/Storage/AbstractDatabaseTable.php b/modules/common/src/Storage/AbstractDatabaseTable.php index e477b179e0..10cf083757 100644 --- a/modules/common/src/Storage/AbstractDatabaseTable.php +++ b/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); diff --git a/modules/datastore/src/Service/ImportService.php b/modules/datastore/src/Service/ImportService.php index 87381d53a7..d8d7294116 100644 --- a/modules/datastore/src/Service/ImportService.php +++ b/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(); diff --git a/modules/metastore/metastore.module b/modules/metastore/metastore.module index bf3d06ca81..8d8541fcf4 100644 --- a/modules/metastore/metastore.module +++ b/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); } /** diff --git a/modules/metastore/src/Factory/MetastoreItemFactoryInterface.php b/modules/metastore/src/Factory/MetastoreItemFactoryInterface.php index e33cb283d4..ef7cf1e6ee 100644 --- a/modules/metastore/src/Factory/MetastoreItemFactoryInterface.php +++ b/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. diff --git a/modules/metastore/src/LifeCycle/LifeCycle.php b/modules/metastore/src/LifeCycle/LifeCycle.php index f1b32df2df..ebd519b9d7 100644 --- a/modules/metastore/src/LifeCycle/LifeCycle.php +++ b/modules/metastore/src/LifeCycle/LifeCycle.php @@ -2,16 +2,17 @@ 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; @@ -19,6 +20,7 @@ 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(); diff --git a/modules/metastore/src/MetastoreItemInterface.php b/modules/metastore/src/MetastoreItemInterface.php index 22d5613bbd..4e9376033f 100644 --- a/modules/metastore/src/MetastoreItemInterface.php +++ b/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(); diff --git a/modules/metastore/src/NodeWrapper/Data.php b/modules/metastore/src/NodeWrapper/Data.php index 2d3cd7980c..f550c80bbc 100644 --- a/modules/metastore/src/NodeWrapper/Data.php +++ b/modules/metastore/src/NodeWrapper/Data.php @@ -3,53 +3,53 @@ namespace Drupal\metastore\NodeWrapper; use Drupal\Core\Entity\EntityInterface; -use Drupal\common\Exception\DataNodeLifeCycleEntityValidationException; +use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\Entity\EntityTypeManagerInterface; +use Drupal\common\Exception\DataNodeLifeCycleEntityValidationException; use Drupal\metastore\MetastoreItemInterface; -use Drupal\node\Entity\Node; +use Drupal\node\NodeInterface; /** * MetastoreItem object that wraps a data node, provides additional methods. + * + * Generate these objects using the factory: + * dkan.metastore.metastore_item_factory. + * + * @see \Drupal\metastore\NodeWrapper\NodeDataFactory::getInstance() */ class Data implements MetastoreItemInterface { /** * Node. * - * @var \Drupal\node\Entity\Node - */ - protected $node; - - /** - * Referenced raw metadata string. - * - * @var string + * @var \Drupal\Core\Entity\EntityInterface */ - protected $rawMetadata; + protected EntityInterface $node; /** * Entity Type Manager. * * @var \Drupal\Core\Entity\EntityTypeManagerInterface */ - private $entityTypeManager; + private EntityTypeManagerInterface $entityTypeManager; /** * Entity Node Storage. * * @var \Drupal\Core\Entity\EntityStorageInterface */ - private $nodeStorage; + private EntityStorageInterface $nodeStorage; /** * Constructor. * * @param \Drupal\Core\Entity\EntityInterface $entity - * A Drupal entity. + * A Drupal entity. Must be a Data node. * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entityTypeManager * Entity Type Manager service. * * @throws \Drupal\common\Exception\DataNodeLifeCycleEntityValidationException + * Thrown when the entity is not a Data node. */ public function __construct(EntityInterface $entity, EntityTypeManagerInterface $entityTypeManager) { $this->validate($entity); @@ -81,6 +81,8 @@ public function getCacheMaxAge() { /** * Private. + * + * @todo Needing to call fix() on every method seems like a code smell. */ private function fix() { $this->fixDataType(); @@ -167,7 +169,7 @@ public function isNew() { * Private. */ private function validate(EntityInterface $entity) { - if (!($entity instanceof Node)) { + if (!($entity instanceof NodeInterface)) { throw new DataNodeLifeCycleEntityValidationException("We only work with nodes."); } @@ -195,6 +197,8 @@ public function getSchemaId() { /** * Private. + * + * @todo Why do we do this? */ private function saveRawMetadata() { // Temporarily save the raw json metadata, for later use. diff --git a/modules/metastore/src/NodeWrapper/NodeDataFactory.php b/modules/metastore/src/NodeWrapper/NodeDataFactory.php index 470ebfb288..32e23a362c 100644 --- a/modules/metastore/src/NodeWrapper/NodeDataFactory.php +++ b/modules/metastore/src/NodeWrapper/NodeDataFactory.php @@ -5,6 +5,7 @@ use Drupal\Core\Entity\EntityRepository; use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\metastore\Factory\MetastoreEntityItemFactoryInterface; +use Drupal\metastore\MetastoreItemInterface; /** * Class NodeDataFactory. @@ -48,25 +49,26 @@ public function __construct(EntityRepository $entityRepository, EntityTypeManage * @param array $config * Optional config from interface, not used. * - * @return Data + * @return \Drupal\metastore\MetastoreItemInterface * Metastore data node object. */ - public function getInstance(string $identifier, array $config = []) { - $dataNode = $this->entityRepository->loadEntityByUuid("node", $identifier); - return new Data($dataNode, $this->entityTypeManager); + public function getInstance(string $identifier, array $config = []): MetastoreItemInterface { + return $this->wrap( + $this->entityRepository->loadEntityByUuid('node', $identifier) + ); } /** * Create a metastore node data object from a node object. * - * @param mixed $dataNode + * @param mixed $input * A data node. * - * @return Data + * @return \Drupal\metastore\MetastoreItemInterface * Metastore data node object. */ - public function wrap($dataNode) { - return new Data($dataNode, $this->entityTypeManager); + public function wrap($input): MetastoreItemInterface { + return new Data($input, $this->entityTypeManager); } /** diff --git a/modules/metastore/src/Reference/ReferenceLookup.php b/modules/metastore/src/Reference/ReferenceLookup.php index d0609fa352..831947409f 100644 --- a/modules/metastore/src/Reference/ReferenceLookup.php +++ b/modules/metastore/src/Reference/ReferenceLookup.php @@ -134,6 +134,9 @@ protected function decodeJsonMetadata(string $json): array { $module_path = $this->moduleHandler->getModule(get_module_name())->getPath(); $legacy_schema_path = $module_path . '/docs/legacy_metadata.json'; // Fetch the legacy metadata schema. + // @todo This file load happens for every metadata item that is processed. + // The schema JSON is then garbage-collected when we leave the scope of + // this function. Find a way to keep and reuse the schema data. $legacy_schema = file_get_contents($legacy_schema_path); // Record metadata identifier. $identifier = $metadata->identifier; diff --git a/modules/metastore/src/Storage/Data.php b/modules/metastore/src/Storage/Data.php index 0788e67d7f..e4b228c65a 100644 --- a/modules/metastore/src/Storage/Data.php +++ b/modules/metastore/src/Storage/Data.php @@ -355,11 +355,14 @@ private function updateExistingEntity(ContentEntityInterface $entity, $data): ?s $entity->{$this->metadataField} = $new_data; // Dkan publishing's default moderation state. + // @todo Honor existing entity's moderation state: + // https://github.com/GetDKAN/dkan/issues/4337 $entity->set('moderation_state', $this->getDefaultModerationState()); if ($entity instanceof RevisionLogInterface) { - $entity->setRevisionLogMessage("Updated on " . (new \DateTimeImmutable())->format(\DateTimeImmutable::ATOM)); - $entity->setRevisionCreationTime(time()); + $time = new \DateTimeImmutable(); + $entity->setRevisionLogMessage("Updated on " . $time->format(\DateTimeImmutable::ATOM)); + $entity->setRevisionCreationTime($time->getTimestamp()); } $entity->save(); diff --git a/modules/metastore/tests/src/Unit/NodeWrapper/DataTest.php b/modules/metastore/tests/src/Unit/NodeWrapper/DataTest.php index 34f2e7aed9..1bebabde50 100644 --- a/modules/metastore/tests/src/Unit/NodeWrapper/DataTest.php +++ b/modules/metastore/tests/src/Unit/NodeWrapper/DataTest.php @@ -17,9 +17,15 @@ use Symfony\Component\DependencyInjection\Container; /** - * Testing the NodeWrapper. + * @coversDefaultClass \Drupal\metastore\NodeWrapper\Data + * @covers \Drupal\metastore\NodeWrapper\Data + * + * @group dkan + * @group metastore + * @group unit */ class DataTest extends TestCase { + public function testGetLatestRevisionGetUsAWrapper() { $node = (new Chain($this)) ->add(Node::class, 'bundle', 'data')