Better error-correction in HarvestUtility

modules/harvest/src/HarvestUtility.php

@@ -197,16 +197,7 @@ public function convertHashTable(string $plan_id) {
    * Outdated tables will be removed.
    */
   public function harvestHashUpdate() {
-    $plan_ids = array_merge(
-      $this->harvestService->getAllHarvestIds(),
-      array_values($this->findOrphanedHarvestDataIds())
-    );
-    foreach ($plan_ids as $plan_id) {
-      $this->logger->notice('Converting hashes for ' . $plan_id);
-      $this->convertHashTable($plan_id);
-      $this->storeFactory->getInstance('harvest_' . $plan_id . '_hashes')
-        ->destruct();
-    }
+    $this->convertTables('hashes');
   }
 
   /**
@@ -231,15 +222,42 @@ public function convertRunTable(string $plan_id) {
    * Outdated tables will be removed.
    */
   public function harvestRunsUpdate() {
+    $this->convertTables('runs');
+  }
+
+  /**
+   * Convert the hashes or runs tables to use entities.
+   *
+   * Outdated tables will be removed.
+   *
+   * @param string $type
+   *   The type of table to convert. Must be one of 'hashes' or 'runs'.
+   */
+  protected function convertTables(string $type) {
+    // Types mapped to conversion methods.
+    $types = [
+      'hashes' => 'convertHashTable',
+      'runs' => 'convertRunTable',
+    ];
+    if (!in_array($type, array_keys($types))) {
+      throw new \InvalidArgumentException('Type must be one of: ' . implode(', ', array_keys($types)));
+    }
     $plan_ids = array_merge(
       $this->harvestService->getAllHarvestIds(),
       array_values($this->findOrphanedHarvestDataIds())
     );
     foreach ($plan_ids as $plan_id) {
-      $this->logger->notice('Converting runs for ' . $plan_id);
-      $this->convertRunTable($plan_id);
-      $this->storeFactory->getInstance('harvest_' . $plan_id . '_runs')
-        ->destruct();
+      $this->logger->notice('Converting ' . $type . ' for ' . $plan_id);
+      try {
+        $this->{$types[$type]}($plan_id);
+        $this->storeFactory->getInstance('harvest_' . $plan_id . '_' . $type)
+          ->destruct();
+      }
+      catch (\Exception $e) {
+        $this->logger->error('Unable to convert ' . $type . ' for ' . $plan_id);
+        $this->logger->error($e->getMessage());
+        $this->logger->debug($e->getTraceAsString());
+      }
     }
   }
 

modules/harvest/tests/src/Kernel/HarvestUtilityTest.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\Tests\harvest\Kernel;
 
+use Drupal\harvest\HarvestUtility;
 use Drupal\KernelTests\KernelTestBase;
 
 /**
@@ -197,4 +198,68 @@ public function testHarvestRunsUpdate() {
     $this->assertEquals('AWESOME', $run_entity->get('extract_status')->getString());
   }
 
+  /**
+   * Ensure an exception during DB handling does not stop further updates.
+   *
+   * @covers ::harvestRunsUpdate
+   */
+  public function testHarvestRunException() {
+    // Mock HarvestUtility so we can have one method throw an exception.
+    $utility = $this->getMockBuilder(HarvestUtility::class)
+      ->setConstructorArgs([
+        $this->container->get('dkan.harvest.service'),
+        $this->container->get('dkan.harvest.storage.database_table'),
+        $this->container->get('dkan.harvest.storage.hashes_database_table'),
+        $this->container->get('dkan.harvest.storage.harvest_run_repository'),
+        $this->container->get('database'),
+        $this->container->get('dkan.harvest.logger_channel'),
+      ])
+      ->onlyMethods(['findOrphanedHarvestDataIds', 'convertRunTable'])
+      ->getMock();
+
+    // findOrphanedHarvestDataIds will return two fake IDs to loop over.
+    $utility->expects($this->atLeast(1))
+      ->method('findOrphanedHarvestDataIds')
+      ->willReturn(['FAKE_PLAN_ID', 'ANOTHER_FAKE_PLAN_ID']);
+    // DB work will throw an exception. We expect it to be called twice because
+    // there are two plan IDs.
+    $utility->expects($this->atLeast(2))
+      ->method('convertRunTable')
+      ->willThrowException(new \Exception('Hello.'));
+
+    $utility->harvestRunsUpdate();
+  }
+
+  /**
+   * Ensure an exception during DB handling does not stop further updates.
+   *
+   * @covers ::harvestHashUpdate
+   */
+  public function testHarvestHashException() {
+    // Mock HarvestUtility so we can mock some methods.
+    $utility = $this->getMockBuilder(HarvestUtility::class)
+      ->setConstructorArgs([
+        $this->container->get('dkan.harvest.service'),
+        $this->container->get('dkan.harvest.storage.database_table'),
+        $this->container->get('dkan.harvest.storage.hashes_database_table'),
+        $this->container->get('dkan.harvest.storage.harvest_run_repository'),
+        $this->container->get('database'),
+        $this->container->get('dkan.harvest.logger_channel'),
+      ])
+      ->onlyMethods(['findOrphanedHarvestDataIds', 'convertHashTable'])
+      ->getMock();
+
+    // findOrphanedHarvestDataIds will return two fake IDs to loop over.
+    $utility->expects($this->atLeast(1))
+      ->method('findOrphanedHarvestDataIds')
+      ->willReturn(['FAKE_PLAN_ID', 'ANOTHER_FAKE_PLAN_ID']);
+    // DB work will throw an exception. We expect it to be called twice because
+    // there are two plan IDs.
+    $utility->expects($this->atLeast(2))
+      ->method('convertHashTable')
+      ->willThrowException(new \Exception('Hello.'));
+
+    $utility->harvestHashUpdate();
+  }
+
 }