Refactored code, updated test

GetDKAN · Dec 23, 2024 · 1921854 · 1921854
1 parent f7bd942
commit 1921854
Show file tree

Hide file tree

Showing 3 changed files with 93 additions and 45 deletions.
diff --git 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
@@ -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
@@ -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(