ImportInfo and ImportJob stores out of sync (#4050)

composer.json

@@ -15,9 +15,9 @@
         "fmizzell/maquina": "^1.1.1",
         "getdkan/contracts": "^1.0.0",
         "getdkan/csv-parser": "^1.3.0",
-        "getdkan/file-fetcher" : "^4.2.2",
+        "getdkan/file-fetcher" : "^5.0.0",
         "getdkan/harvest": "^1.0.0",
-        "getdkan/procrastinator": "^4.0.0",
+        "getdkan/procrastinator": "^5.0.0",
         "getdkan/rooted-json-data": "^0.1",
         "guzzlehttp/guzzle" : "^6.5.8 || ^7.4.5",
         "ilbee/csv-response": "^1.1.1",

modules/common/tests/src/Functional/FileFetcherFactoryTest.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\Tests\common\Kernel\FileFetcher;
 
+use Drupal\common\FileFetcher\DkanFileFetcher;
 use Drupal\common\FileFetcher\FileFetcherFactory;
 use Drupal\KernelTests\KernelTestBase;
 use FileFetcher\FileFetcher;
@@ -63,6 +64,7 @@ public function testOurRemote($use_existing, $remote_class) {
     ];
     $ff = $factory->getInstance('identifier', $config);
     $this->assertInstanceOf(FileFetcher::class, $ff);
+    $this->assertInstanceOf(DkanFileFetcher::class, $ff);
 
     // Make sure we have the correct processor class that corresponds to our
     // config.

modules/datastore/datastore.services.yml

@@ -6,7 +6,6 @@ services:
       - '@dkan.datastore.service.factory.import'
       - '@queue'
       - '@dkan.common.job_store'
-      - '@dkan.datastore.import_info_list'
       - '@dkan.datastore.service.resource_processor.dictionary_enforcer'
 
   dkan.datastore.service.post_import:
@@ -101,9 +100,10 @@ services:
   dkan.datastore.import_info:
     class: \Drupal\datastore\Service\Info\ImportInfo
     arguments:
-      - '@dkan.common.job_store'
       - '@dkan.datastore.service.resource_localizer'
       - '@dkan.datastore.service.factory.import'
+      - '@dkan.metastore.resource_mapper'
+      - '@dkan.datastore.service'
 
   dkan.datastore.import_info_list:
     class: \Drupal\datastore\Service\Info\ImportInfoList

modules/datastore/modules/datastore_mysql_import/src/Factory/MysqlImportFactory.php

@@ -9,9 +9,7 @@
 use Drupal\datastore_mysql_import\Storage\MySqlDatabaseTableFactory;
 
 /**
- * Importer factory.
- *
- * @codeCoverageIgnore
+ * Mysql importer factory.
  */
 class MysqlImportFactory implements ImportFactoryInterface {
 
@@ -29,13 +27,6 @@ class MysqlImportFactory implements ImportFactoryInterface {
    */
   private $databaseTableFactory;
 
-  /**
-   * Services array. Not really needed, following FactoryInterface.
-   *
-   * @var array
-   */
-  private $services = [];
-
   /**
    * Constructor.
    */
@@ -50,20 +41,14 @@ public function __construct(JobStoreFactory $jobStoreFactory, MySqlDatabaseTable
    * @inheritdoc
    */
   public function getInstance(string $identifier, array $config = []) {
-
-    if (!isset($config['resource'])) {
+    $resource = $config['resource'] ?? FALSE;
+    if (!$resource) {
       throw new \Exception("config['resource'] is required");
     }
 
-    $resource = $config['resource'];
-
-    if (!isset($this->services[$identifier])) {
-      $this->services[$identifier] = new ImportService($resource, $this->jobStoreFactory, $this->databaseTableFactory);
-    }
-
-    $this->services[$identifier]->setImporterClass(MysqlImport::class);
-
-    return $this->services[$identifier];
+    $importer = new ImportService($resource, $this->jobStoreFactory, $this->databaseTableFactory);
+    $importer->setImporterClass(MysqlImport::class);
+    return $importer;
   }
 
 }

modules/datastore/modules/datastore_mysql_import/src/Service/MysqlImport.php

@@ -41,6 +41,7 @@ class MysqlImport extends ImportJob {
    *   Configuration options.
    */
   protected function __construct(string $identifier, $storage, array $config = NULL) {
+    // Ensure we can check if the data has already been imported.
     if (!($config['storage'] instanceof ImportedItemInterface)) {
       throw new \Exception('Storage must be an instance of ' . ImportedItemInterface::class);
     }
@@ -62,7 +63,7 @@ protected function runIt() {
     // If the storage table already exists, we already performed an import and
     // can stop here.
     if ($this->dataStorage->hasBeenImported()) {
-      $this->getResult()->setStatus(Result::DONE);
+      $this->setStatus(Result::DONE);
       return NULL;
     }
 
@@ -101,7 +102,7 @@ protected function runIt() {
 
     Database::setActiveConnection();
 
-    $this->getResult()->setStatus(Result::DONE);
+    $this->setStatus(Result::DONE);
     return NULL;
   }
 

modules/datastore/src/Controller/ImportController.php

@@ -3,13 +3,14 @@
 namespace Drupal\datastore\Controller;
 
 use Drupal\common\DataResource;
-use Symfony\Component\DependencyInjection\ContainerInterface;
-use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
 use Drupal\common\JsonResponseTrait;
 use Drupal\Component\Uuid\Uuid;
+use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
 use Drupal\datastore\DatastoreService;
+use Drupal\datastore\Service\Info\ImportInfoList;
 use Drupal\metastore\MetastoreApiResponse;
 use Drupal\metastore\Reference\ReferenceLookup;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\HttpFoundation\Request;
 
 /**
@@ -27,19 +28,42 @@ class ImportController implements ContainerInjectionInterface {
    *
    * @var \Drupal\datastore\DatastoreService
    */
-  protected $datastoreService;
+  protected DatastoreService $datastoreService;
+
+  /**
+   * Metastore API response service.
+   *
+   * @var \Drupal\metastore\MetastoreApiResponse
+   */
+  protected MetastoreApiResponse $metastoreApiResponse;
+
+  /**
+   * Reference lookup service.
+   *
+   * @var \Drupal\metastore\Reference\ReferenceLookup
+   */
+  protected ReferenceLookup $referenceLookup;
+
+  /**
+   * Import info list service.
+   *
+   * @var \Drupal\datastore\Service\Info\ImportInfoList
+   */
+  protected ImportInfoList $importInfoList;
 
   /**
    * Api constructor.
    */
   public function __construct(
     DatastoreService $datastoreService,
     MetastoreApiResponse $metastoreApiResponse,
-    ReferenceLookup $referenceLookup
+    ReferenceLookup $referenceLookup,
+    ImportInfoList $importInfoList
   ) {
     $this->datastoreService = $datastoreService;
     $this->metastoreApiResponse = $metastoreApiResponse;
     $this->referenceLookup = $referenceLookup;
+    $this->importInfoList = $importInfoList;
   }
 
   /**
@@ -49,7 +73,8 @@ public static function create(ContainerInterface $container) {
     return new ImportController(
       $container->get('dkan.datastore.service'),
       $container->get('dkan.metastore.api_response'),
-      $container->get('dkan.metastore.reference_lookup')
+      $container->get('dkan.metastore.reference_lookup'),
+      $container->get('dkan.datastore.import_info_list')
     );
   }
 
@@ -211,7 +236,7 @@ public function deleteMultiple(Request $request) {
    */
   public function list(Request $request) {
     try {
-      $data = $this->datastoreService->list();
+      $data = $this->importInfoList->buildList();
       return $this->metastoreApiResponse->cachedJsonResponse($data, 200, ['distribution'], $request->query);
     }
     catch (\Exception $e) {

modules/datastore/src/DatastoreResource.php

@@ -4,6 +4,10 @@
 
 /**
  * Basic datastore resource class.
+ *
+ * Always generate this object using DataResource::getDatastoreResource().
+ *
+ * @see \Drupal\common\DataResource::getDatastoreResource()
  */
 class DatastoreResource implements \JsonSerializable {
 

modules/datastore/src/DatastoreService.php

@@ -4,16 +4,16 @@
 
 use Drupal\common\DataResource;
 use Drupal\common\Storage\JobStoreFactory;
+use Drupal\datastore\Service\ImportService;
 use Procrastinator\Result;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
 use Drupal\Core\Queue\QueueFactory;
 use Drupal\datastore\Plugin\QueueWorker\ImportJob;
-use Drupal\datastore\Service\ResourceLocalizer;
 use Drupal\datastore\Service\Factory\ImportFactoryInterface;
-use Drupal\datastore\Service\Info\ImportInfoList;
-use FileFetcher\FileFetcher;
+use Drupal\datastore\Service\ResourceLocalizer;
 use Drupal\datastore\Service\ResourceProcessor\DictionaryEnforcer;
+use FileFetcher\FileFetcher;
 
 /**
  * Main services for the datastore.
@@ -30,7 +30,7 @@ class DatastoreService implements ContainerInjectionInterface {
   /**
    * Datastore import factory class.
    *
-   * @var \Drupal\datastore\Service\Factory\ImportFactoryInterface
+   * @var \Drupal\datastore\Service\Factory\ImportServiceFactory
    */
   private $importServiceFactory;
 
@@ -64,7 +64,6 @@ public static function create(ContainerInterface $container) {
       $container->get('dkan.datastore.service.factory.import'),
       $container->get('queue'),
       $container->get('dkan.common.job_store'),
-      $container->get('dkan.datastore.import_info_list'),
       $container->get('dkan.datastore.service.resource_processor.dictionary_enforcer')
     );
   }
@@ -80,8 +79,6 @@ public static function create(ContainerInterface $container) {
    *   Queue factory service.
    * @param \Drupal\common\Storage\JobStoreFactory $jobStoreFactory
    *   Jobstore factory service.
-   * @param \Drupal\datastore\Service\Info\ImportInfoList $importInfoList
-   *   Import info list service.
    * @param \Drupal\datastore\Service\ResourceProcessor\DictionaryEnforcer $dictionaryEnforcer
    *   Dictionary Enforcer object.
    */
@@ -90,14 +87,12 @@ public function __construct(
     ImportFactoryInterface $importServiceFactory,
     QueueFactory $queue,
     JobStoreFactory $jobStoreFactory,
-    ImportInfoList $importInfoList,
     DictionaryEnforcer $dictionaryEnforcer
   ) {
     $this->queue = $queue;
     $this->resourceLocalizer = $resourceLocalizer;
     $this->importServiceFactory = $importServiceFactory;
     $this->jobStoreFactory = $jobStoreFactory;
-    $this->importInfoList = $importInfoList;
     $this->dictionaryEnforcer = $dictionaryEnforcer;
   }
 
@@ -150,7 +145,9 @@ public function import(string $identifier, bool $deferred = FALSE, $version = NU
   private function doImport($resource) {
     $importService = $this->getImportService($resource);
     $importService->import();
-    return [$this->getLabelFromObject($importService) => $importService->getResult()];
+    return [
+      $this->getLabelFromObject($importService) => $importService->getImporter()->getResult(),
+    ];
   }
 
   /**
@@ -197,8 +194,10 @@ private function getResource($identifier, $version) {
   /**
    * Getter.
    */
-  public function getImportService(DataResource $resource) {
-    return $this->importServiceFactory->getInstance($resource->getUniqueIdentifier(), ['resource' => $resource]);
+  public function getImportService(DataResource $resource): ImportService {
+    return $this->importServiceFactory->getInstance(
+      $resource->getUniqueIdentifier(), ['resource' => $resource]
+    );
   }
 
   /**
@@ -237,16 +236,6 @@ public function drop(string $identifier, ?string $version = NULL, bool $local_re
     }
   }
 
-  /**
-   * Get a list of all stored importers and filefetchers, and their status.
-   *
-   * @return array
-   *   The importer list object.
-   */
-  public function list() {
-    return $this->importInfoList->buildList();
-  }
-
   /**
    * Summary.
    */

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

@@ -219,10 +219,10 @@ protected function runIt() {
     $this->store();
 
     if ($this->getBytesProcessed() >= $size) {
-      $this->getResult()->setStatus(Result::DONE);
+      $this->setStatus(Result::DONE);
     }
     else {
-      $this->getResult()->setStatus(Result::STOPPED);
+      $this->setStatus(Result::STOPPED);
     }
 
     return $this->getResult();
@@ -258,8 +258,8 @@ protected function assertTextFile(string $filename) {
    *   Updated result object.
    */
   protected function setResultError($message): Result {
-    $this->getResult()->setStatus(Result::ERROR);
     $this->getResult()->setError($message);
+    $this->setStatus(Result::ERROR);
     return $this->getResult();
   }
 
@@ -291,16 +291,16 @@ protected function parseAndStore($filename, $maximumExecutionTime) {
       $chunk = fread($h, self::BYTES_PER_CHUNK);
 
       if (!$chunk) {
-        $this->getResult()->setStatus(Result::DONE);
+        $this->setStatus(Result::DONE);
         $this->parser->finish();
         break;
       }
       $chunk = Encoding::toUTF8($chunk);
       $this->parser->feed($chunk);
       $chunksProcessed++;
 
-      $this->store();
       $this->setStateProperty('chunksProcessed', $chunksProcessed);
+      $this->store();
     }
     fclose($h);
   }

modules/datastore/src/Service/Factory/ImportServiceFactory.php

@@ -52,12 +52,7 @@ public function getInstance(string $identifier, array $config = []) {
     }
 
     $resource = $config['resource'];
-
-    if (!isset($this->services[$identifier])) {
-      $this->services[$identifier] = new ImportService($resource, $this->jobStoreFactory, $this->databaseTableFactory);
-    }
-
-    return $this->services[$identifier];
+    return new ImportService($resource, $this->jobStoreFactory, $this->databaseTableFactory);
   }
 
 }