From 192185406600580951a503af9ccfe8fc30b443b0 Mon Sep 17 00:00:00 2001 From: Kaise Lafrai Date: Mon, 23 Dec 2024 17:06:09 -0500 Subject: [PATCH] Refactored code, updated test --- .../PostImportResourceProcessor.php | 16 +++ modules/datastore/src/Service/PostImport.php | 118 +++++++++++------- .../src/Kernel/Service/PostImportTest.php | 4 +- 3 files changed, 93 insertions(+), 45 deletions(-) diff --git a/modules/datastore/src/Plugin/QueueWorker/PostImportResourceProcessor.php b/modules/datastore/src/Plugin/QueueWorker/PostImportResourceProcessor.php index bba4675290..ad34a83cab 100644 --- a/modules/datastore/src/Plugin/QueueWorker/PostImportResourceProcessor.php +++ b/modules/datastore/src/Plugin/QueueWorker/PostImportResourceProcessor.php @@ -22,6 +22,11 @@ */ class PostImportResourceProcessor extends QueueWorkerBase implements ContainerFactoryPluginInterface { + /** + * The PostImport service. + * + * @var \Drupal\datastore\Service\PostImport + */ protected PostImport $postImport; /** @@ -32,6 +37,16 @@ class PostImportResourceProcessor extends QueueWorkerBase implements ContainerFa protected $configFactory; /** + * Constructor for PostImportResourceProcessor. + * + * @param array $configuration + * A configuration array containing information about the plugin instance. + * @param string $pluginId + * The plugin_id for the plugin instance. + * @param mixed $pluginDefinition + * The plugin implementation definition. + * @param \Drupal\datastore\Service\PostImport $postImport + * The PostImport service. * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory * The config.factory service. */ @@ -67,4 +82,5 @@ public function processItem($data) { $result = $this->postImport->processResource($data); $result->storeResult(); } + } diff --git a/modules/datastore/src/Service/PostImport.php b/modules/datastore/src/Service/PostImport.php index 83f0e5f7bb..862e38dd76 100644 --- a/modules/datastore/src/Service/PostImport.php +++ b/modules/datastore/src/Service/PostImport.php @@ -132,66 +132,98 @@ public function __construct( * @param \Drupal\common\DataResource $resource * DKAN Resource. * - * @return PostImportResult + * @return \Drupal\datastore\PostImportResult * The post import result service. */ public function processResource(DataResource $resource): PostImportResult { - $latestResource = $this->resourceMapper->get($resource->getIdentifier()); - - // Exit if resource no longer exists. - if (!$latestResource) { - $this->logger->notice('Cancelling resource processing; resource no longer exists.'); - return $this->createPostImportResult('error', 'Cancelling resource processing; resource no longer exists.', $resource); - } - - // Exit if resource has changed. - if ($resource->getVersion() !== $latestResource->getVersion()) { - $this->logger->notice('Cancelling resource processing; resource has changed.'); - return $this->createPostImportResult('error', 'Cancelling resource processing; resource has changed.', $resource); - } - - // Exit if data dictionary is disabled. - if ($this->dataDictionaryDiscovery->getDataDictionaryMode() === DataDictionaryDiscoveryInterface::MODE_NONE) { - $this->logger->notice('Data-Dictionary Disabled'); - return $this->createPostImportResult('N/A', 'Data-Dictionary Disabled', $resource); + if ($result = $this->validateResource($resource)) { + return $result; } try { - $processors = $this->resourceProcessorCollector->getResourceProcessors(); - - array_map(fn($processor) => $processor->process($resource), $processors); - + $this->processResourceProcessors($resource); $this->logger->notice('Post import job for resource @id completed.', ['@id' => $resource->getIdentifier()]); $this->invalidateCacheTags($resource->getIdentifier()); - return $this->createPostImportResult('done', NULL, $resource); } catch (ResourceDoesNotHaveDictionary $e) { $this->logger->notice($e->getMessage()); - return $this->createPostImportResult('done', 'Resource does not have a data dictionary.', $resource); } catch (\Exception $e) { - $identifier = $resource->getIdentifier(); - - if ($this->configFactory->get('datastore.settings')->get('drop_datastore_on_post_import_error')) { - try { - $this->drop($identifier, NULL, FALSE); - $this->logger->notice('Successfully dropped the datastore for resource @identifier due to a post import error. Visit the Datastore Import Status dashboard for details.', [ - '@identifier' => $identifier, - ]); - } - catch (\Exception $e) { - $this->logger->error($e->getMessage()); - } - } - - $this->logger->error($e->getMessage()); + $this->handleProcessingError($resource, $e); return $this->createPostImportResult('error', $e->getMessage(), $resource); } + } + +/** + * Handle errors during resource processing. + * + * @param \Drupal\common\DataResource $resource + * DKAN Resource. + * @param \Exception $exception + * The caught exception. + */ +private function handleProcessingError(DataResource $resource, \Exception $exception): void { + $identifier = $resource->getIdentifier(); + + if ($this->configFactory->get('datastore.settings')->get('drop_datastore_on_post_import_error')) { + try { + $this->drop($identifier, NULL, FALSE); + $this->logger->notice('Successfully dropped the datastore for resource @identifier due to a post import error. Visit the Datastore Import Status dashboard for details.', [ + '@identifier' => $identifier, + ]); + } catch (\Exception $dropException) { + $this->logger->error($dropException->getMessage()); + } + } + $this->logger->error($exception->getMessage()); +} + +/** + * Process resource. + * + * @param \Drupal\common\DataResource $resource + * DKAN Resource. + * + * @throws \Exception + */ +private function processResourceProcessors(DataResource $resource): void { + $processors = $this->resourceProcessorCollector->getResourceProcessors(); + array_map(fn($processor) => $processor->process($resource), $processors); +} + +/** + * Validation checks before processing resource. + * + * @param \Drupal\common\DataResource $resource + * DKAN Resource. + * + * @return \Drupal\datastore\PostImportResult|null + * Post import result if validation fails, or NULL if validation passes. + */ +private function validateResource(DataResource $resource): ?PostImportResult { + $latestResource = $this->resourceMapper->get($resource->getIdentifier()); + + if (!$latestResource) { + $this->logger->notice('Cancelling resource processing; resource no longer exists.'); + return $this->createPostImportResult('error', 'Cancelling resource processing; resource no longer exists.', $resource); } + if ($resource->getVersion() !== $latestResource->getVersion()) { + $this->logger->notice('Cancelling resource processing; resource has changed.'); + return $this->createPostImportResult('error', 'Cancelling resource processing; resource has changed.', $resource); + } + + if ($this->dataDictionaryDiscovery->getDataDictionaryMode() === DataDictionaryDiscoveryInterface::MODE_NONE) { + $this->logger->notice('Data-Dictionary Disabled'); + return $this->createPostImportResult('N/A', 'Data-Dictionary Disabled', $resource); + } + + return NULL; +} + /** * Create the PostImportResult object. * @@ -201,8 +233,8 @@ public function processResource(DataResource $resource): PostImportResult { * Error messages retrieved during the post import process. * @param \Drupal\common\DataResource $resource * The DKAN resource being imported. - * - * @return PostImportResult + * + * @return \Drupal\datastore\PostImportResult * The post import result service. */ protected function createPostImportResult($status, $message, DataResource $resource): PostImportResult { @@ -302,7 +334,7 @@ public function removeJobStatus($resourceIdentifier): bool { public function drop($resourceIdentifier): bool { try { $this->datastoreService->drop($resourceIdentifier, NULL, FALSE); - return true; + return TRUE; } catch (\Exception $e) { throw $e; diff --git a/modules/datastore/tests/src/Kernel/Service/PostImportTest.php b/modules/datastore/tests/src/Kernel/Service/PostImportTest.php index a130930705..e61a439dad 100644 --- a/modules/datastore/tests/src/Kernel/Service/PostImportTest.php +++ b/modules/datastore/tests/src/Kernel/Service/PostImportTest.php @@ -191,7 +191,7 @@ public function testProcessResourceErrorWithFailingDrop() { ->getMock(); $datastore_service->expects($this->once()) ->method('drop') - ->willThrowException(new \Exception('our test message')); + ->willThrowException(new \Exception('drop error')); $this->container->set('dkan.datastore.service', $datastore_service); $post_import = new PostImport( @@ -208,7 +208,7 @@ public function testProcessResourceErrorWithFailingDrop() { $result = $post_import->processResource($resource); $this->assertEquals( - 'our test message', + 'Attempted to retrieve a sitewide data dictionary, but none was set.', $result->getPostImportMessage() ); $this->assertEquals(