#4075: Catch Guzzle Exception to avoid breaking harvest/dataset entry

modules/metastore/metastore.services.yml

@@ -41,6 +41,7 @@ services:
       - '@config.factory'
       - '@dkan.metastore.storage'
       - '@dkan.metastore.url_generator'
+      - '@http_client'
     calls:
       - [setLoggerFactory, ['@logger.factory']]
 

modules/metastore/src/Reference/Referencer.php

@@ -12,7 +12,8 @@
 use Drupal\metastore\MetastoreService;
 use Drupal\metastore\ResourceMapper;
 
-use GuzzleHttp\Client as GuzzleClient;
+use GuzzleHttp\Client;
+use GuzzleHttp\Exception\GuzzleException;
 
 /**
  * Metastore referencer service.
@@ -26,7 +27,7 @@ class Referencer {
    *
    * @var string
    */
-  protected const DEFAULT_MIME_TYPE = 'text/plain';
+  public const DEFAULT_MIME_TYPE = 'text/plain';
 
   /**
    * Storage factory interface service.
@@ -42,18 +43,27 @@ class Referencer {
    */
   public MetastoreUrlGenerator $metastoreUrlGenerator;
 
+  /**
+   * Guzzle HTTP client.
+   *
+   * @var \GuzzleHttp\Client
+   */
+  private Client $httpClient;
+
   /**
    * Constructor.
    */
   public function __construct(
     ConfigFactoryInterface $configService,
     FactoryInterface $storageFactory,
-    MetastoreUrlGenerator $metastoreUrlGenerator
+    MetastoreUrlGenerator $metastoreUrlGenerator,
+    Client $httpClient
   ) {
     $this->setConfigService($configService);
     $this->storageFactory = $storageFactory;
     $this->setLoggerFactory(\Drupal::service('logger.factory'));
     $this->metastoreUrlGenerator = $metastoreUrlGenerator;
+    $this->httpClient = $httpClient;
   }
 
   /**
@@ -342,8 +352,12 @@ private function getRemoteMimeType(string $downloadUrl): ?string {
 
     // Perform HTTP Head request against the supplied URL in order to determine
     // the content type of the remote resource.
-    $client = new GuzzleClient();
-    $response = $client->head($downloadUrl);
+    try {
+      $response = $this->httpClient->head($downloadUrl);
+    }
+    catch (GuzzleException $exception) {
+      return $mime_type;
+    }
     // Extract the full value of the content type header.
     $content_type = $response->getHeader('Content-Type');
     // Attempt to extract the mime type from the content type header.
@@ -366,7 +380,7 @@ private function getRemoteMimeType(string $downloadUrl): ?string {
    * @todo Update the UI to set mediaType when a format is selected.
    */
   private function getMimeType($distribution): string {
-    $mimeType = "text/plain";
+    $mimeType = self::DEFAULT_MIME_TYPE;
 
     // If we have a mediaType set, use that.
     if (isset($distribution->mediaType)) {
@@ -384,7 +398,8 @@ private function getMimeType($distribution): string {
     elseif (isset($distribution->downloadURL)) {
       // Determine whether the supplied distribution has a local or remote
       // resource.
-      $is_local = $distribution->downloadURL !== UrlHostTokenResolver::hostify($distribution->downloadURL);
+      $is_local = $distribution->downloadURL !==
+        UrlHostTokenResolver::hostify($distribution->downloadURL);
       $mimeType = $is_local ?
         $this->getLocalMimeType($distribution->downloadURL) :
         $this->getRemoteMimeType($distribution->downloadURL);

modules/metastore/tests/src/Unit/Reference/ReferencerTest.php

@@ -26,6 +26,7 @@
 use Drupal\node\Entity\Node;
 use Drupal\node\NodeStorage;
 
+use GuzzleHttp\Client;
 use GuzzleHttp\Exception\ConnectException;
 use MockChain\Chain;
 use MockChain\Options;
@@ -35,6 +36,13 @@
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\RequestStack;
 
+/**
+ * @covers \Drupal\metastore\Reference\Referencer
+ * @coversDefaultClass \Drupal\metastore\Reference\Referencer
+ *
+ * @group dkan
+ * @group metastore
+ */
 class ReferencerTest extends TestCase {
 
   /**
@@ -110,8 +118,12 @@ private function mockReferencer($existing = TRUE) {
       ->add(MetastoreUrlGenerator::class, 'uriFromUrl', 'dkan://metastore/schemas/data-dictionary/items/111')
       ->getMock();
 
-    $referencer = new Referencer($configService, $storageFactory, $urlGenerator);
-    return $referencer;
+    return new Referencer(
+      $configService,
+      $storageFactory,
+      $urlGenerator,
+      new Client()
+    );
   }
 
   private function getContainer() {
@@ -375,6 +387,10 @@ private function getData(string $downloadUrl, string $mediaType = NULL): object
 
   /**
    * Test the remote/local file mime type detection logic.
+   *
+   * @covers ::getLocalMimeType
+   * @covers ::getMimeType
+   * @covers ::getRemoteMimeType
    */
   public function testMimeTypeDetection(): void {
     // Initialize mock node class.
@@ -430,7 +446,12 @@ public function getMimeType() { return ReferencerTest::MIME_TYPE; }
       ->add(MetastoreUrlGenerator::class, 'uriFromUrl', '')
       ->getMock();
 
-    $referencer = new Referencer($configService, $storageFactory, $urlGenerator);
+    $referencer = new Referencer(
+      $configService,
+      $storageFactory,
+      $urlGenerator,
+      new Client()
+    );
 
     // Test Mime Type detection using the resource `mediaType` property.
     $data = $this->getData(self::HOST . '/' . self::FILE_PATH, self::MIME_TYPE);
@@ -444,10 +465,11 @@ public function getMimeType() { return ReferencerTest::MIME_TYPE; }
     $data = $this->getData('https://dkan-default-content-files.s3.amazonaws.com/phpunit/district_centerpoints_small.csv');
     $referencer->reference($data);
     $this->assertEquals(self::MIME_TYPE, $container_chain->getStoredInput('resource')[0]->getMimeType(), 'Unable to fetch MIME type for remote file');
-    // Test Mime Type detection on a invalid remote file path.
+    // Test Mime Type detection on a invalid remote file path. Defaults to
+    // text/plain.
     $data = $this->getData('http://invalid');
-    $this->expectException(ConnectException::class);
     $referencer->reference($data);
+    $this->assertEquals(Referencer::DEFAULT_MIME_TYPE, $container_chain->getStoredInput('resource')[0]->getMimeType(), 'Did not use default MIME type for inaccessible remote file.');
   }
 
   /**
@@ -480,7 +502,16 @@ public function testDistributionHandlingDataDict($distribution, $describedBy) {
       )
       ->getMock();
 
-    $referencer = new Referencer($configService, $storageFactory, $urlGenerator);
+    $http_client = $this->getMockBuilder(Client::class)
+      ->disableOriginalConstructor()
+      ->getMock();
+
+    $referencer = new Referencer(
+      $configService,
+      $storageFactory,
+      $urlGenerator,
+      $http_client
+    );
 
     if ($describedBy instanceof \Exception) {
       $this->expectException(get_class($describedBy));