From f5b0f0767706fb9c73b8227d304cb6dba53db7a6 Mon Sep 17 00:00:00 2001 From: Clayton Liddell Date: Tue, 23 Nov 2021 08:57:29 -0600 Subject: [PATCH] Only retrieve necessary IDs for import dashboard (#3710) --- modules/datastore/datastore.routing.yml | 4 +- .../src/Controller/DashboardController.php | 104 ++++++++---------- .../Controller/DashboardControllerTest.php | 15 +-- modules/metastore/src/Service.php | 30 +++++ modules/metastore/src/Storage/Data.php | 58 +++++++--- .../metastore/tests/src/Unit/ServiceTest.php | 34 ++++++ .../tests/src/Unit/Storage/DataTest.php | 49 +++++++++ 7 files changed, 210 insertions(+), 84 deletions(-) diff --git a/modules/datastore/datastore.routing.yml b/modules/datastore/datastore.routing.yml index cb088da534..7cf44dfac9 100644 --- a/modules/datastore/datastore.routing.yml +++ b/modules/datastore/datastore.routing.yml @@ -159,8 +159,8 @@ datastore.datasets_import_status_dashboard: path: '/admin/dkan/dataset-status/{harvestId}' methods: [ GET ] defaults: - _controller: \Drupal\datastore\Controller\DashboardController::datasetsImportStatus - _title_callback: \Drupal\datastore\Controller\DashboardController::datasetsImportStatusTitle + _controller: \Drupal\datastore\Controller\DashboardController::buildDatasetsImportStatusTable + _title_callback: \Drupal\datastore\Controller\DashboardController::buildDatasetsImportStatusTitle harvestId: NULL requirements: _permission: 'dkan.harvest.dashboard' diff --git a/modules/datastore/src/Controller/DashboardController.php b/modules/datastore/src/Controller/DashboardController.php index 9211eb991e..2895c89d89 100644 --- a/modules/datastore/src/Controller/DashboardController.php +++ b/modules/datastore/src/Controller/DashboardController.php @@ -100,7 +100,7 @@ public function __construct( /** * Create controller object from dependency injection container. */ - public static function create(ContainerInterface $container) { + public static function create(ContainerInterface $container): self { return new static( $container->get('dkan.harvest.service'), $container->get('dkan.common.dataset_info'), @@ -110,28 +110,14 @@ public static function create(ContainerInterface $container) { } /** - * Datasets information. + * Build datasets import status table. */ - public function datasetsImportStatus($harvestId) { - if (!empty($harvestId)) { - $harvestLoad = $this->getHarvestLoadStatus($harvestId); - $datasets = array_keys($harvestLoad); - } - else { - $harvestLoad = []; - foreach ($this->harvest->getAllHarvestIds() as $harvestId) { - $harvestLoad += $this->getHarvestLoadStatus($harvestId); - } - $datasets = $this->getAllDatasetUuids(); - } - - $rows = $this->buildDatasetRows($datasets, $harvestLoad); - + public function buildDatasetsImportStatusTable(?string $harvestId): array { return [ 'table' => [ '#theme' => 'table', '#header' => self::DATASET_HEADERS, - '#rows' => $this->pagerArray($rows, $this->itemsPerPage), + '#rows' => $this->buildDatasetRows($harvestId), '#attributes' => ['class' => 'dashboard-datasets'], '#attached' => ['library' => ['harvest/style']], '#empty' => 'No datasets found', @@ -143,21 +129,9 @@ public function datasetsImportStatus($harvestId) { } /** - * Gets all dataset uuids from metadata. - * - * @return array - * Dataset uuids array. + * Build datasets import status table title. */ - private function getAllDatasetUuids() : array { - return array_map(function ($datasetMetadata) { - return $datasetMetadata->{"$.identifier"}; - }, $this->metastore->getAll('dataset')); - } - - /** - * Datasets information. Title callback. - */ - public function datasetsImportStatusTitle($harvestId) { + public function buildDatasetsImportStatusTitle(?string $harvestId) { if (!empty($harvestId)) { return $this->t("Datastore Import Status. Harvest %harvest", ['%harvest' => $harvestId]); } @@ -167,7 +141,7 @@ public function datasetsImportStatusTitle($harvestId) { /** * Private. */ - private function getHarvestLoadStatus($harvestId): array { + private function getHarvestLoadStatus(?string $harvestId): array { $runIds = $this->harvest->getAllHarvestRunInfo($harvestId); $runId = end($runIds); @@ -181,28 +155,58 @@ private function getHarvestLoadStatus($harvestId): array { /** * Builds dataset rows array to be themed as a table. * - * @param array $datasets - * Dataset uuids array. - * @param array $harvestLoad - * Harvest statuses by dataset array. + * @param array|null $harvestId + * Harvest ID for which to generate dataset rows. * * @return array * Table rows. */ - private function buildDatasetRows(array $datasets, array $harvestLoad) { + private function buildDatasetRows(?string $harvestId): array { $rows = []; - foreach ($datasets as $datasetId) { + foreach ($this->getDatasetsWithHarvestId($harvestId) as $datasetId => $status) { $datasetInfo = $this->datasetInfo->gather($datasetId); if (empty($datasetInfo['latest_revision'])) { continue; } - $harvestStatus = isset($harvestLoad[$datasetId]) ? $harvestLoad[$datasetId] : 'N/A'; - $datasetRow = $this->buildDatasetRow($datasetInfo, $harvestStatus); + $datasetRow = $this->buildDatasetRow($datasetInfo, $status); $rows = array_merge($rows, $datasetRow); } return $rows; } + /** + * Retrieve datasets and import status belonging to the given harvest ID. + * + * @param string|null $harvestId + * Harvest ID which fetched datasets should belong to. + * + * @return string[] + * Dataset import statuses keyed by their dataset IDs. + */ + protected function getDatasetsWithHarvestId(?string $harvestId): array { + if (!empty($harvestId)) { + $harvestLoad = $this->getHarvestLoadStatus($harvestId); + $datasets = array_keys($harvestLoad); + $total = count($datasets); + $currentPage = $this->pagerManager->createPager($total, $this->itemsPerPage)->getCurrentPage(); + + $chunks = array_chunk($datasets, $this->itemsPerPage) ?: [[]]; + $datasets = $chunks[$currentPage]; + } + else { + $harvestLoad = []; + foreach ($this->harvest->getAllHarvestIds() as $harvestId) { + $harvestLoad += $this->getHarvestLoadStatus($harvestId); + } + $total = $this->metastore->count('dataset'); + $currentPage = $this->pagerManager->createPager($total, $this->itemsPerPage)->getCurrentPage(); + $datasets = $this->metastore->getRangeUuids('dataset', $currentPage, $this->itemsPerPage); + $datasets = array_replace(array_fill_keys($datasets, 'N/A'), $harvestLoad); + } + + return $datasets; + } + /** * May build 2 rows if data has both published and draft version. */ @@ -289,22 +293,4 @@ private function percentCell(int $percent) { ]; } - /** - * Returns pager array. - * - * @param array $items - * Table rows. - * @param int $itemsPerPage - * Items per page. - * - * @return array - * Table rows chunk. - */ - private function pagerArray(array $items, int $itemsPerPage) : array { - $total = count($items); - $currentPage = $this->pagerManager->createPager($total, $itemsPerPage)->getCurrentPage(); - $chunks = array_chunk($items, $itemsPerPage); - return !empty($chunks) ? $chunks[$currentPage] : []; - } - } diff --git a/modules/datastore/tests/src/Unit/Controller/DashboardControllerTest.php b/modules/datastore/tests/src/Unit/Controller/DashboardControllerTest.php index 175895da89..672dab3f59 100644 --- a/modules/datastore/tests/src/Unit/Controller/DashboardControllerTest.php +++ b/modules/datastore/tests/src/Unit/Controller/DashboardControllerTest.php @@ -40,7 +40,7 @@ public function testNoDatasets() { $controller = DashboardController::create($container->getMock()); - $response = $controller->datasetsImportStatus('test'); + $response = $controller->buildDatasetsImportStatusTable('test'); $json = json_encode($response); $strings = array_merge(DashboardController::DATASET_HEADERS,); @@ -85,7 +85,7 @@ public function testDataset() { $controller = DashboardController::create($container->getMock()); - $response = $controller->datasetsImportStatus('test'); + $response = $controller->buildDatasetsImportStatusTable('test'); $json = json_encode($response); $strings = array_merge( @@ -99,7 +99,7 @@ public function testDataset() { $this->assertStringContainsString($string, $json); } - $title = (string) $controller->datasetsImportStatusTitle('test'); + $title = (string) $controller->buildDatasetsImportStatusTitle('test'); $this->assertEquals('Datastore Import Status. Harvest test', $title); } @@ -157,14 +157,15 @@ public function testAllDatasets() { $container = $this->getCommonMockChain() ->add(Harvest::class, 'getAllHarvestRunInfo', [$time]) - ->add(MetastoreService::class, 'getAll', $metastoreGetAllDatasets) + ->add(MetastoreService::class, 'count', 2) + ->add(MetastoreService::class, 'getRangeUuids', [$dataset1Info['latest_revision']['uuid'], $nonHarvestDatasetInfo['latest_revision']['uuid']]) ->add(DatasetInfo::class, 'gather', $datasetInfoOptions); \Drupal::setContainer($container->getMock()); $controller = DashboardController::create($container->getMock()); - $response = $controller->datasetsImportStatus(NULL); + $response = $controller->buildDatasetsImportStatusTable(NULL); // Assert that there are both datasets: harvested and non-harvested. $this->assertEquals(2, count($response["table"]['#rows'])); @@ -177,7 +178,7 @@ public function testAllDatasets() { $this->assertEquals('Non-Harvest Dataset', $response["table"]["#rows"][1][1]); $this->assertEquals('N/A', $response["table"]["#rows"][1][4]["data"]); - $title = (string) $controller->datasetsImportStatusTitle(NULL); + $title = (string) $controller->buildDatasetsImportStatusTitle(NULL); $this->assertEquals('Datastore Import Status', $title); } @@ -206,7 +207,7 @@ public function testDatasetNoDistribution() { $controller = DashboardController::create($container->getMock()); - $response = $controller->datasetsImportStatus('test'); + $response = $controller->buildDatasetsImportStatusTable('test'); $this->assertEmpty($response["table"]["#rows"][0][7]["data"]["#rows"]); } diff --git a/modules/metastore/src/Service.php b/modules/metastore/src/Service.php index 4bf35964ff..e14f8733cf 100644 --- a/modules/metastore/src/Service.php +++ b/modules/metastore/src/Service.php @@ -113,6 +113,36 @@ private function getStorage(string $schema_id): MetastoreStorageInterface { return $this->storages[$schema_id]; } + /** + * Count objects of the given schema ID. + * + * @param string $schema_id + * The schema ID to be counted. + * + * @return int + * Object count. + */ + public function count(string $schema_id): int { + return $this->getStorage($schema_id)->count(); + } + + /** + * Get range of object UUIDs of the given schema ID. + * + * @param string $schema_id + * The schema ID to be counted. + * @param int $start + * Schema object offset. + * @param int $length + * Number of objects to fetch. + * + * @return string[] + * Range of object UUIDs of the given schema_id. + */ + public function getRangeUuids(string $schema_id, int $start, int $length): array { + return $this->getStorage($schema_id)->retrieveRangeUuids($start, $length); + } + /** * Get all. * diff --git a/modules/metastore/src/Storage/Data.php b/modules/metastore/src/Storage/Data.php index cbefb03bfb..8a810e62e8 100644 --- a/modules/metastore/src/Storage/Data.php +++ b/modules/metastore/src/Storage/Data.php @@ -87,6 +87,21 @@ private function setSchema($schemaId) { $this->schemaId = $schemaId; } + /** + * Count objects of this metastore's schema ID. + * + * @return int + * Object count. + */ + public function count(): int { + return $this->entityStorage->getQuery() + ->accessCheck(FALSE) + ->condition('type', $this->bundle) + ->condition('field_data_type', $this->schemaId) + ->count() + ->execute(); + } + /** * Inherited. * @@ -110,13 +125,36 @@ public function retrieveAll(): array { return $all; } + /** + * Get range of object UUIDs of this metastore's schema ID. + * + * @param int $start + * Schema object offset. + * @param int $length + * Number of objects to fetch. + * + * @return string[] + * Range of object UUIDs of the given schema_id. + */ + public function retrieveRangeUuids(int $start, int $length): array { + $ids = $this->entityStorage->getQuery() + ->accessCheck(FALSE) + ->condition('type', $this->bundle) + ->condition('field_data_type', $this->schemaId) + ->range($start, $length) + ->execute(); + + return array_map(function ($entity) { + return $entity->uuid(); + }, $this->entityStorage->loadMultiple($ids)); + } + /** * Inherited. * * {@inheritdoc}. */ public function retrieveRange($start, $length): array { - $entity_ids = $this->entityStorage->getQuery() ->accessCheck(FALSE) ->condition('type', $this->bundle) @@ -125,8 +163,7 @@ public function retrieveRange($start, $length): array { ->execute(); $all = []; - foreach ($entity_ids as $nid) { - $entity = $this->entityStorage->load($nid); + foreach ($this->entityStorage->loadMultiple($entity_ids) as $entity) { if ($entity->get('moderation_state')->getString() === 'published') { $all[] = $entity->get('field_json_metadata')->getString(); } @@ -299,7 +336,7 @@ private function updateExistingEntity(EntityInterface $entity, $data) { // Dkan publishing's default moderation state. $entity->set('moderation_state', $this->getDefaultModerationState()); - $entity->setRevisionLogMessage("Updated on " . $this->formattedTimestamp()); + $entity->setRevisionLogMessage("Updated on " . (new \DateTimeImmutable())->format(\DateTimeImmutable::ATOM)); $entity->setRevisionCreationTime(time()); $entity->save(); @@ -338,7 +375,7 @@ private function createNewEntity(string $uuid, $data) { 'field_json_metadata' => json_encode($data), ] ); - $entity->setRevisionLogMessage("Created on " . $this->formattedTimestamp()); + $entity->setRevisionLogMessage("Created on " . (new \DateTimeImmutable())->format(\DateTimeImmutable::ATOM)); $entity->save(); @@ -409,17 +446,6 @@ private function htmlPurifier(string $input) { return $filter->purify($input); } - /** - * Returns the current time, formatted. - * - * @return string - * Current timestamp, formatted. - */ - private function formattedTimestamp() : string { - $now = new \DateTime('now'); - return $now->format(\DateTime::ATOM); - } - /** * Return the default moderation state of our custom dkan_publishing workflow. * diff --git a/modules/metastore/tests/src/Unit/ServiceTest.php b/modules/metastore/tests/src/Unit/ServiceTest.php index a5b5141391..299341e046 100644 --- a/modules/metastore/tests/src/Unit/ServiceTest.php +++ b/modules/metastore/tests/src/Unit/ServiceTest.php @@ -318,6 +318,40 @@ public function testPublishMissingObjectExpection() { $service->publish('dataset', "foobar"); } + /** + * Test \Drupal\metastore\Service::count() method. + */ + public function testCount(): void { + // Set constant which should be returned by the ::count() method. + $count = 5; + + // Create mock chain for testing ::count() method. + $container = self::getCommonMockChain($this) + ->add(NodeData::class, 'count', $count); + + // Create metastore service object. + $service = Service::create($container->getMock()); + // Ensure count matches return value. + $this->assertEquals($count, $service->count('test')); + } + + /** + * Test \Drupal\metastore\Service::getRangeUuids() method. + */ + public function testGetRangeUuids(): void { + // Set constant which should be returned by the ::getRangeUuids() method. + $uuids = ['a', 'b', 'c']; + + // Create mock chain for testing ::getRangeUuids() method. + $container = self::getCommonMockChain($this) + ->add(NodeData::class, 'retrieveRangeUuids', $uuids); + + // Create metastore service object. + $service = Service::create($container->getMock()); + // Ensure count matches return value. + $this->assertEquals($uuids, $service->getRangeUuids('test', 1, 5)); + } + /** * */ diff --git a/modules/metastore/tests/src/Unit/Storage/DataTest.php b/modules/metastore/tests/src/Unit/Storage/DataTest.php index 7e69ab79e3..58efaab286 100644 --- a/modules/metastore/tests/src/Unit/Storage/DataTest.php +++ b/modules/metastore/tests/src/Unit/Storage/DataTest.php @@ -66,9 +66,58 @@ private function getEtmChain() { ->add(NodeStorage::class, 'getQuery', QueryInterface::class) ->add(QueryInterface::class, 'accessCheck', QueryInterface::class) ->add(QueryInterface::class, 'condition', QueryInterface::class) + ->add(QueryInterface::class, 'count', QueryInterface::class) + ->add(QueryInterface::class, 'range', QueryInterface::class) ->add(QueryInterface::class, 'execute', ['1']) ->add(NodeStorage::class, 'getLatestRevisionId', '2') ->addd('loadRevision', Node::class); } + /** + * Test \Drupal\metastore\Storage\Data::count() method. + */ + public function testCount(): void { + // Set constant which should be returned by the ::count() method. + $count = 5; + + // Create mock chain for testing ::count() method. + $etmMock = $this->getEtmChain() + ->add(QueryInterface::class, 'execute', $count) + ->getMock(); + + // Create Data object. + $nodeData = new NodeData('dataset', $etmMock); + // Ensure count matches return value. + $this->assertEquals($count, $nodeData->count()); + } + + /** + * Test \Drupal\metastore\Storage\Data::retrieveRangeUuids() method. + */ + public function testRetrieveRangeUuids(): void { + // Generate dataset nodes for testing ::retrieveRangeUuids(). + $nodes = []; + $uuids = []; + + for ($i = 0; $i < 5; $i ++) { + $nodes[$i] = new class { + private $uuid; + public function uuid() { + return isset($this->uuid) ? $this->uuid : $this->uuid = uniqid(); + } + }; + $uuids[$i] = $nodes[$i]->uuid(); + } + + // Create mock chain for testing ::retrieveRangeUuids() method. + $etmMock = $this->getEtmChain() + ->add(NodeStorage::class, 'loadMultiple', $nodes) + ->getMock(); + + // Create Data object. + $nodeData = new NodeData('dataset', $etmMock); + // Ensure the returned uuids match those belonging to the generated nodes. + $this->assertEquals($uuids, $nodeData->retrieveRangeUuids(1, 5)); + } + }