Add option to drop datastore if there is an error on post_import (#4311)

cypress/integration/09_admin_links.spec.js

@@ -10,11 +10,11 @@ context('Administration pages', () => {
     const links = [
       'Datasets',
       'Datastore Import Status',
-      'Datastore settings',
+      'Datastore Settings',
       'Data Dictionary',
       'Harvests',
-      'Metastore settings',
-      'Resources'
+      'Metastore Settings',
+      'Resource Settings'
     ]
 
     cy.visit(`${baseurl}/admin`)
@@ -25,19 +25,19 @@ context('Administration pages', () => {
     })
   })
 
-  it('Admin can access the Metastore settings.', () => {
+  it('Admin can access the Metastore Settings.', () => {
     cy.get('.toolbar-icon-system-admin-dkan').contains('DKAN').next('.toolbar-menu').then($el=>{
         cy.wrap($el).invoke('show')
-        cy.wrap($el).contains('Metastore settings')
+        cy.wrap($el).contains('Metastore Settings')
     })
     cy.visit(baseurl + "/admin/dkan/properties")
     cy.get('.option').should('contain.text', 'Distribution (distribution)')
   })
 
-  it('Admin can access the Datastore settings.', () => {
+  it('Admin can access the Datastore Settings.', () => {
     cy.get('.toolbar-icon-system-admin-dkan').contains('DKAN').next('.toolbar-menu').then($el=>{
         cy.wrap($el).invoke('show')
-        cy.wrap($el).contains('Datastore settings')
+        cy.wrap($el).contains('Datastore Settings')
     })
     cy.visit(baseurl + "/admin/dkan/datastore")
     cy.get('label[for="edit-rows-limit"]').should('have.text', 'Rows limit')

modules/datastore/config/schema/datastore.schema.yml

@@ -14,3 +14,5 @@ datastore.settings:
         type: string
     delete_local_resource:
       type: boolean
+    drop_datastore_on_post_import_error:
+      type: boolean

modules/datastore/datastore.links.menu.yml

@@ -1,12 +1,12 @@
 datastore.settings_form:
-  title: Datastore settings
+  title: Datastore Settings
   description: Datastore settings.
   parent: system.admin_dkan
   route_name: datastore.settings
   weight: 12
 
 datastore.dkan_resource_settings:
-  title: Resources
+  title: Resource Settings
   description: Resource options.
   parent: system.admin_dkan
   route_name: datastore.dkan_resource_settings

modules/datastore/datastore.routing.yml

@@ -150,7 +150,7 @@ datastore.sql_endpoint.post.api:
 datastore.dkan_resource_settings:
   path: '/admin/dkan/resources'
   defaults:
-    _title: 'Resources'
+    _title: 'Resource Settings'
     _form: 'Drupal\datastore\Form\ResourceSettingsForm'
   requirements:
     _permission: 'administer site configuration'

modules/datastore/src/Form/ResourceSettingsForm.php

@@ -54,6 +54,16 @@ public function buildForm(array $form, FormStateInterface $form_state) {
       '#default_value' => $this->config('datastore.settings')->get('delete_local_resource'),
       '#description' => $this->t('Delete local copy of remote files after the datastore import is complete'),
     ];
+    $form['drop_datastore_on_post_import_error'] = [
+      '#type' => 'checkbox',
+      '#title' => $this->t('Drop the datastore table if the post import queue reports an error.'),
+      '#default_value' => $this->config('datastore.settings')->get('drop_datastore_on_post_import_error'),
+      '#description' => $this->t('The datastore import queue brings in all columns as strings. The post import
+      queue will alter the table according to the data dictionary, if there is a problem during this step the
+      error will be posted to the Datastore Import Status dashboard, and the datastore table will keep all
+      data typed as strings. Check this box if you prefer that the table be dropped if there is a problem
+      in the post import stage.'),
+    ];
     $form['resource_perspective_display'] = [
       '#type' => 'select',
       '#title' => $this->t('Resource download url display'),
@@ -76,6 +86,7 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
       ->set('purge_table', $form_state->getValue('purge_table'))
       ->set('purge_file', $form_state->getValue('purge_file'))
       ->set('delete_local_resource', $form_state->getValue('delete_local_resource'))
+      ->set('drop_datastore_on_post_import_error', $form_state->getValue('drop_datastore_on_post_import_error'))
       ->save();
     $this->config('metastore.settings')
       ->set('resource_perspective_display', $form_state->getValue('resource_perspective_display'))

modules/datastore/src/Plugin/QueueWorker/PostImportResourceProcessor.php

@@ -2,11 +2,13 @@
 
 namespace Drupal\datastore\Plugin\QueueWorker;
 
+use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
 use Drupal\Core\Queue\QueueWorkerBase;
 use Drupal\common\DataResource;
 use Drupal\datastore\DataDictionary\AlterTableQueryBuilderInterface;
 use Drupal\datastore\PostImportResult;
+use Drupal\datastore\DatastoreService;
 use Drupal\datastore\Service\PostImport;
 use Drupal\datastore\Service\ResourceProcessor\ResourceDoesNotHaveDictionary;
 use Drupal\datastore\Service\ResourceProcessorCollector;
@@ -30,6 +32,13 @@
  */
 class PostImportResourceProcessor extends QueueWorkerBase implements ContainerFactoryPluginInterface {
 
+  /**
+   * The datastore.settings config.
+   *
+   * @var \Drupal\Core\Config\ImmutableConfig
+   */
+  protected $config;
+
   /**
    * A logger channel for this plugin.
    *
@@ -51,6 +60,13 @@ class PostImportResourceProcessor extends QueueWorkerBase implements ContainerFa
    */
   protected ResourceProcessorCollector $resourceProcessorCollector;
 
+  /**
+   * The datastore service.
+   *
+   * @var \Drupal\datastore\DatastoreService
+   */
+  protected DatastoreService $datastoreService;
+
   /**
    * The PostImport service.
    *
@@ -81,6 +97,8 @@ class PostImportResourceProcessor extends QueueWorkerBase implements ContainerFa
    *   The plugin_id for the plugin instance.
    * @param mixed $plugin_definition
    *   The plugin implementation definition.
+   * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory
+   *   The config.factory service.
    * @param \Drupal\datastore\DataDictionary\AlterTableQueryBuilderInterface $alter_table_query_builder
    *   The alter table query factory service.
    * @param \Psr\Log\LoggerInterface $logger_channel
@@ -89,6 +107,8 @@ class PostImportResourceProcessor extends QueueWorkerBase implements ContainerFa
    *   The metastore resource mapper service.
    * @param \Drupal\datastore\Service\ResourceProcessorCollector $processor_collector
    *   The resource processor collector service.
+   * @param \Drupal\datastore\DatastoreService $datastoreService
+   *   The resource datastore service.
    * @param \Drupal\datastore\Service\PostImport $post_import
    *   The post import service.
    * @param \Drupal\metastore\DataDictionary\DataDictionaryDiscoveryInterface $data_dictionary_discovery
@@ -100,18 +120,22 @@ public function __construct(
     array $configuration,
     $plugin_id,
     $plugin_definition,
+    ConfigFactoryInterface $configFactory,
     AlterTableQueryBuilderInterface $alter_table_query_builder,
     LoggerInterface $logger_channel,
     ResourceMapper $resource_mapper,
     ResourceProcessorCollector $processor_collector,
+    DatastoreService $datastoreService,
     PostImport $post_import,
     DataDictionaryDiscoveryInterface $data_dictionary_discovery,
     ReferenceLookup $referenceLookup
   ) {
     parent::__construct($configuration, $plugin_id, $plugin_definition);
+    $this->config = $configFactory;
     $this->logger = $logger_channel;
     $this->resourceMapper = $resource_mapper;
     $this->resourceProcessorCollector = $processor_collector;
+    $this->datastoreService = $datastoreService;
     $this->postImport = $post_import;
     $this->dataDictionaryDiscovery = $data_dictionary_discovery;
     // Set the timeout for database connections to the queue lease time.
@@ -130,10 +154,12 @@ public static function create(ContainerInterface $container, array $configuratio
       $configuration,
       $plugin_id,
       $plugin_definition,
+      $container->get('config.factory'),
       $container->get('dkan.datastore.data_dictionary.alter_table_query_builder.mysql'),
       $container->get('dkan.datastore.logger_channel'),
       $container->get('dkan.metastore.resource_mapper'),
       $container->get('dkan.datastore.service.resource_processor_collector'),
+      $container->get('dkan.datastore.service'),
       $container->get('dkan.datastore.service.post_import'),
       $container->get('dkan.metastore.data_dictionary_discovery'),
       $container->get('dkan.metastore.reference_lookup'),
@@ -145,13 +171,28 @@ public static function create(ContainerInterface $container, array $configuratio
    */
   public function processItem($data) {
     $postImportResult = $this->postImportProcessItem($data);
+    $drop_config = $this->config->get('datastore.settings')
+      ->get('drop_datastore_on_post_import_error');
+
     if ($postImportResult->getPostImportStatus() === 'done') {
       $this->invalidateCacheTags(DataResource::buildUniqueIdentifier(
         $data->getIdentifier(),
         $data->getVersion(),
         DataResource::DEFAULT_SOURCE_PERSPECTIVE
       ));
     }
+    if ($postImportResult->getPostImportStatus() === 'error' && $drop_config) {
+      $identifier = $data->getIdentifier();
+      try {
+        $this->datastoreService->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());
+      }
+    }
     // Store the results of the PostImportResult object.
     $postImportResult->storeResult();
   }

modules/datastore/tests/src/Kernel/Plugin/QueueWorker/PostImportResourceProcessorTest.php

@@ -1,9 +1,14 @@
 <?php
 
+declare(strict_types=1);
+
 namespace Drupal\Tests\datastore\Kernel\Plugin\QueueWorker;
 
 use Drupal\common\DataResource;
+use Drupal\Core\Logger\LoggerChannelInterface;
+use Drupal\datastore\DatastoreService;
 use Drupal\datastore\Plugin\QueueWorker\PostImportResourceProcessor;
+use Drupal\datastore\PostImportResult;
 use Drupal\datastore\Service\ResourceProcessor\DictionaryEnforcer;
 use Drupal\datastore\Service\ResourceProcessor\ResourceDoesNotHaveDictionary;
 use Drupal\KernelTests\KernelTestBase;
@@ -90,4 +95,162 @@ public function testPostImportProcessItemNoDictionary() {
     );
   }
 
+  /**
+   * @covers ::processItem
+   */
+  public function testProcessItem() {
+    $data_identifier = 'test_identifier';
+
+    $this->config('datastore.settings')
+      ->set('drop_datastore_on_post_import_error', TRUE)
+      ->save();
+
+    // Our error result.
+    $error_result = $this->getMockBuilder(PostImportResult::class)
+      ->disableOriginalConstructor()
+      ->onlyMethods(['getPostImportStatus', 'storeResult'])
+      ->getMock();
+    $error_result->expects($this->any())
+      ->method('getPostImportStatus')
+      ->willReturn('error');
+    $error_result->expects($this->once())
+      ->method('storeResult');
+
+    // Mock a logger to expect error logging.
+    $logger = $this->getMockBuilder(LoggerChannelInterface::class)
+      ->onlyMethods(['error', 'notice'])
+      ->getMockForAbstractClass();
+    // Never expect an error.
+    $logger->expects($this->never())
+      ->method('error');
+    // Expect one notice.
+    $logger->expects($this->once())
+      ->method('notice')
+      ->with(
+        'Successfully dropped the datastore for resource @identifier due to a post import error. Visit the Datastore Import Status dashboard for details.',
+        ['@identifier' => $data_identifier],
+      );
+    $this->container->set('dkan.datastore.logger_channel', $logger);
+
+    // Datastore service will always succeed. Mocked so we don't have to deal
+    // with dropping an actual datastore.
+    $datastore_service = $this->getMockBuilder(DatastoreService::class)
+      ->disableOriginalConstructor()
+      ->onlyMethods(['drop'])
+      ->getMock();
+    $datastore_service->expects($this->once())
+      ->method('drop');
+    // Put the service into the service container.
+    $this->container->set('dkan.datastore.service', $datastore_service);
+
+    // Return our error result.
+    $post_import_resource_processor = $this->getMockBuilder(PostImportResourceProcessor::class)
+      ->setConstructorArgs([
+        [],
+        '',
+        ['cron' => ['lease_time' => 10800]],
+        $this->container->get('config.factory'),
+        $this->container->get('dkan.datastore.data_dictionary.alter_table_query_builder.mysql'),
+        $this->container->get('dkan.datastore.logger_channel'),
+        $this->container->get('dkan.metastore.resource_mapper'),
+        $this->container->get('dkan.datastore.service.resource_processor_collector'),
+        $this->container->get('dkan.datastore.service'),
+        $this->container->get('dkan.datastore.service.post_import'),
+        $this->container->get('dkan.metastore.data_dictionary_discovery'),
+        $this->container->get('dkan.metastore.reference_lookup'),
+      ])
+      ->onlyMethods(['postImportProcessItem'])
+      ->getMock();
+    $post_import_resource_processor->expects($this->once())
+      ->method('postImportProcessItem')
+      ->willReturn($error_result);
+
+    // Data we'll pass to our method under test.
+    $data = $this->getMockBuilder(DataResource::class)
+      ->disableOriginalConstructor()
+      ->onlyMethods(['getIdentifier'])
+      ->getMock();
+    $data->expects($this->once())
+      ->method('getIdentifier')
+      ->willReturn($data_identifier);
+
+    $post_import_resource_processor->processItem($data);
+  }
+
+  /**
+   * @covers ::processItem
+   */
+  public function testProcessItemExceptionPath() {
+    $this->config('datastore.settings')
+      ->set('drop_datastore_on_post_import_error', TRUE)
+      ->save();
+
+    // Our error result.
+    $error_result = $this->getMockBuilder(PostImportResult::class)
+      ->disableOriginalConstructor()
+      ->onlyMethods(['getPostImportStatus', 'storeResult'])
+      ->getMock();
+    $error_result->expects($this->any())
+      ->method('getPostImportStatus')
+      ->willReturn('error');
+    $error_result->expects($this->once())
+      ->method('storeResult');
+
+    // Mock a logger to expect error logging.
+    $logger = $this->getMockBuilder(LoggerChannelInterface::class)
+      ->onlyMethods(['error', 'notice'])
+      ->getMockForAbstractClass();
+    // Expect an error.
+    $logger->expects($this->once())
+      ->method('error');
+    // Expect no notices.
+    $logger->expects($this->never())
+      ->method('notice');
+    $this->container->set('dkan.datastore.logger_channel', $logger);
+
+    // Datastore service rigged to explode.
+    $datastore_service = $this->getMockBuilder(DatastoreService::class)
+      ->disableOriginalConstructor()
+      ->onlyMethods(['drop'])
+      ->getMock();
+    $datastore_service->expects($this->once())
+      ->method('drop')
+      ->willThrowException(new \Exception('our test message'));
+    // Put the service into the service container.
+    $this->container->set('dkan.datastore.service', $datastore_service);
+
+    // Return our error result.
+    $post_import_resource_processor = $this->getMockBuilder(PostImportResourceProcessor::class)
+      ->setConstructorArgs([
+        [],
+        '',
+        ['cron' => ['lease_time' => 10800]],
+        $this->container->get('config.factory'),
+        $this->container->get('dkan.datastore.data_dictionary.alter_table_query_builder.mysql'),
+        $this->container->get('dkan.datastore.logger_channel'),
+        $this->container->get('dkan.metastore.resource_mapper'),
+        $this->container->get('dkan.datastore.service.resource_processor_collector'),
+        $this->container->get('dkan.datastore.service'),
+        $this->container->get('dkan.datastore.service.post_import'),
+        $this->container->get('dkan.metastore.data_dictionary_discovery'),
+        $this->container->get('dkan.metastore.reference_lookup'),
+      ])
+      ->onlyMethods(['postImportProcessItem'])
+      ->getMock();
+    $post_import_resource_processor->expects($this->once())
+      ->method('postImportProcessItem')
+      ->willReturn($error_result);
+
+    // Data we'll pass to our method under test.
+    $data = $this->getMockBuilder(DataResource::class)
+      ->disableOriginalConstructor()
+      ->onlyMethods(['getIdentifier'])
+      ->getMock();
+    $data->expects($this->once())
+      ->method('getIdentifier')
+      ->willReturn('test');
+
+    $post_import_resource_processor->processItem($data);
+  }
+
 }