From cbdd8709647aaaa1492f45e20cabe2b42f3c9d59 Mon Sep 17 00:00:00 2001 From: Dan Feder Date: Wed, 21 Dec 2022 13:35:49 -0500 Subject: [PATCH 1/4] Revert "Switch to simple built-in FileFetcher on import (#3881)" This reverts commit edd187cc20f6162fe9b723a8a64617f4b8ce349c. --- composer.json | 1 + modules/common/common.services.yml | 5 + modules/common/src/FileFetcher/Factory.php | 56 ++++++ modules/common/tests/src/Traits/CleanUp.php | 4 +- .../tests/src/Unit/Storage/JobStoreTest.php | 18 +- modules/datastore/datastore.services.yml | 1 + .../src/Plugin/QueueWorker/FileFetcherJob.php | 165 ------------------ .../src/Plugin/QueueWorker/ImportJob.php | 2 +- modules/datastore/src/Service.php | 11 +- .../datastore/src/Service/Info/ImportInfo.php | 3 +- .../src/Service/Info/ImportInfoList.php | 4 +- .../src/Service/ResourceLocalizer.php | 28 ++- modules/datastore/tests/data/tiny.csv | 2 - .../datastore/tests/src/Mock/Container.php | 2 + .../Plugin/QueueWorker/FileFetcherJobTest.php | 102 ----------- .../Unit/Service/Info/ImportInfoListTest.php | 4 +- .../Unit/Service/ResourceLocalizerTest.php | 24 ++- .../datastore/tests/src/Unit/ServiceTest.php | 5 +- .../tests/src/Unit/Storage/JobStoreTest.php | 18 +- modules/sample_content/sample_content.json | 26 +-- 20 files changed, 150 insertions(+), 331 deletions(-) create mode 100644 modules/common/src/FileFetcher/Factory.php delete mode 100644 modules/datastore/src/Plugin/QueueWorker/FileFetcherJob.php delete mode 100644 modules/datastore/tests/data/tiny.csv delete mode 100644 modules/datastore/tests/src/Unit/Plugin/QueueWorker/FileFetcherJobTest.php diff --git a/composer.json b/composer.json index 65b48b56fc..926bd0defe 100644 --- a/composer.json +++ b/composer.json @@ -18,6 +18,7 @@ "fmizzell/maquina": "^1.1.0", "getdkan/contracts": "^1.0.0", "getdkan/csv-parser": "^1.2.3", + "getdkan/file-fetcher" : "^4.1.0", "getdkan/harvest": "^1.0.0", "getdkan/json-schema-provider": "^0.1.2", "getdkan/locker": "^1.1.0", diff --git a/modules/common/common.services.yml b/modules/common/common.services.yml index dfda5b0944..d3760afea9 100644 --- a/modules/common/common.services.yml +++ b/modules/common/common.services.yml @@ -24,6 +24,11 @@ services: factory: entity_type.manager:getStorage arguments: ['node'] + dkan.common.file_fetcher: + class: \Drupal\common\FileFetcher\Factory + arguments: + - '@dkan.common.job_store' + dkan.common.dataset_info: class: \Drupal\common\DatasetInfo calls: diff --git a/modules/common/src/FileFetcher/Factory.php b/modules/common/src/FileFetcher/Factory.php new file mode 100644 index 0000000000..31ef1b43e3 --- /dev/null +++ b/modules/common/src/FileFetcher/Factory.php @@ -0,0 +1,56 @@ + TRUE, + ]; + + /** + * Constructor. + */ + public function __construct(JobStoreFactory $factory) { + $this->factory = $factory; + } + + /** + * Inherited. + * + * @inheritdoc + */ + public function getInstance(string $identifier, array $config = []) { + $config = array_merge($this->configDefault, $config); + return FileFetcher::get($identifier, $this->getFileFetcherJobStore(), $config); + } + + /** + * Private. + */ + private function getFileFetcherJobStore() { + /** @var \Drupal\common\Storage\JobStoreFactory $jobStoreFactory */ + $jobStoreFactory = $this->factory; + return $jobStoreFactory->getInstance(FileFetcher::class); + } + +} diff --git a/modules/common/tests/src/Traits/CleanUp.php b/modules/common/tests/src/Traits/CleanUp.php index 9ddf633aa8..0a9dda8eb0 100644 --- a/modules/common/tests/src/Traits/CleanUp.php +++ b/modules/common/tests/src/Traits/CleanUp.php @@ -3,7 +3,7 @@ namespace Drupal\Tests\common\Traits; use Drupal\node\Entity\Node; -use Drupal\datastore\Plugin\QueueWorker\FileFetcherJob; +use FileFetcher\FileFetcher; /** * @@ -50,7 +50,7 @@ private function removeAllFileFetchingJobs() { $jobStoreFactory = \Drupal::service('dkan.common.job_store'); /** @var \Drupal\common\Storage\JobStore $jobStore */ - $jobStore = $jobStoreFactory->getInstance(FileFetcherJob::class); + $jobStore = $jobStoreFactory->getInstance(FileFetcher::class); foreach ($jobStore->retrieveAll() as $id) { $jobStore->remove($id); } diff --git a/modules/common/tests/src/Unit/Storage/JobStoreTest.php b/modules/common/tests/src/Unit/Storage/JobStoreTest.php index 0126d5a323..38d20e8622 100644 --- a/modules/common/tests/src/Unit/Storage/JobStoreTest.php +++ b/modules/common/tests/src/Unit/Storage/JobStoreTest.php @@ -9,9 +9,9 @@ use Drupal\Core\Database\Schema; use Drupal\Core\Database\StatementWrapper; use Drupal\common\Storage\JobStore; -use Drupal\datastore\Plugin\QueueWorker\FileFetcherJob; use Contracts\Mock\Storage\Memory; +use FileFetcher\FileFetcher; use MockChain\Chain; use MockChain\Sequence; use PHPUnit\Framework\TestCase; @@ -30,7 +30,7 @@ public function testConstruction() { ->add(Connection::class, "schema", Schema::class) ->add(Schema::class, "tableExists", FALSE); - $jobStore = new JobStore(FileFetcherJob::class, $chain->getMock()); + $jobStore = new JobStore(FileFetcher::class, $chain->getMock()); $this->assertTrue(is_object($jobStore)); } @@ -59,8 +59,8 @@ public function testRetrieve() { ->add(Connection::class, 'query', StatementWrapper::class) ->add(StatementWrapper::class, 'fetchAll', $fieldInfo); - $jobStore = new JobStore(FileFetcherJob::class, $chain->getMock()); - $this->assertEquals($job_data, $jobStore->retrieve("1", FileFetcherJob::class)); + $jobStore = new JobStore(FileFetcher::class, $chain->getMock()); + $this->assertEquals($job_data, $jobStore->retrieve("1", FileFetcher::class)); } /** @@ -90,7 +90,7 @@ public function testRetrieveAll() { ->add(Connection::class, 'query', StatementWrapper::class) ->add(StatementWrapper::class, 'fetchAll', $sequence); - $jobStore = new JobStore(FileFetcherJob::class, $chain->getMock()); + $jobStore = new JobStore(FileFetcher::class, $chain->getMock()); $this->assertTrue(is_array($jobStore->retrieveAll())); } @@ -127,7 +127,7 @@ public function testStore() { ->add(StatementWrapper::class, 'fetchAll', $fieldInfo) ->getMock(); - $jobStore = new JobStore(FileFetcherJob::class, $connection); + $jobStore = new JobStore(FileFetcher::class, $connection); $this->assertEquals("1", $jobStore->store(json_encode($jobObject), "1")); } @@ -151,16 +151,16 @@ public function testRemove() { ->add(StatementWrapper::class, 'fetchAll', $fieldInfo) ->getMock(); - $jobStore = new JobStore(FileFetcherJob::class, $connection); + $jobStore = new JobStore(FileFetcher::class, $connection); - $this->assertEquals("", $jobStore->remove("1", FileFetcherJob::class)); + $this->assertEquals("", $jobStore->remove("1", FileFetcher::class)); } /** * Private. */ private function getFileFetcher() { - return FileFetcherJob::get("1", new Memory(), ["filePath" => "file://" . __DIR__ . "/../../data/countries.csv"]); + return FileFetcher::get("1", new Memory(), ["filePath" => "file://" . __DIR__ . "/../../data/countries.csv"]); } } diff --git a/modules/datastore/datastore.services.yml b/modules/datastore/datastore.services.yml index 05906fbc57..6ff5b2ad1a 100644 --- a/modules/datastore/datastore.services.yml +++ b/modules/datastore/datastore.services.yml @@ -17,6 +17,7 @@ services: class: \Drupal\datastore\Service\ResourceLocalizer arguments: - '@dkan.metastore.resource_mapper' + - '@dkan.common.file_fetcher' - '@dkan.common.drupal_files' - '@dkan.common.job_store' diff --git a/modules/datastore/src/Plugin/QueueWorker/FileFetcherJob.php b/modules/datastore/src/Plugin/QueueWorker/FileFetcherJob.php deleted file mode 100644 index fdabf1c195..0000000000 --- a/modules/datastore/src/Plugin/QueueWorker/FileFetcherJob.php +++ /dev/null @@ -1,165 +0,0 @@ - $config['filePath'], - 'total_bytes' => 0, - 'total_bytes_copied' => 0, - 'temporary' => FALSE, - 'temporary_directory' => $config['temporaryDirectory'] ?? '/tmp', - ]; - - $this->getResult()->setData(json_encode($state)); - } - - /** - * {@inheritdoc} - */ - protected function runIt() { - $state = $this->setupState($this->getState()); - $this->getResult()->setData(json_encode($state)); - $info = $this->copy($this->getState(), $this->getResult(), $this->getTimeLimit()); - $this->setState($info['state']); - return $info['result']; - } - - /** - * Set up the job state. - * - * @param array $state - * Incoming state array. - * - * @return array - * Modified state array. - */ - protected function setupState(array $state): array { - $state['total_bytes'] = PHP_INT_MAX; - $state['temporary'] = TRUE; - $state['destination'] = $this->getTemporaryFilePath($state); - - return $state; - } - - /** - * Get temporary file path, depending on flag keep_original_filename value. - * - * @param array $state - * State. - * - * @return string - * Temporary file path. - */ - private function getTemporaryFilePath(array $state): string { - $file_name = basename($state['source']); - return "{$state['temporary_directory']}/{$file_name}"; - } - - /** - * Copy the file to local storage. - * - * @param array $state - * State array. - * @param \Procrastinator\Result $result - * Job result object. - * - * @return array - * Array with two elements: state and result. - */ - public function copy(array $state, Result $result): array { - if (stream_is_local($state['source'])) { - return $this->copyLocal($state, $result); - } - else { - return $this->copyRemote($state, $result); - } - } - - /** - * Copy local file to proper local storage. - * - * @param array $state - * State array. - * @param \Procrastinator\Result $result - * Job result object. - * - * @return array - * Array with two elements: state and result. - */ - protected function copyLocal(array $state, Result $result): array { - $this->ensureCreatingForWriting($state['destination']); - if (copy($state['source'], $state['destination'])) { - $result->setStatus(Result::DONE); - } - else { - throw new \Exception("File copy failed."); - } - $state['total_bytes_copied'] = $state['total_bytes'] = filesize($state['destination']); - return ['state' => $state, 'result' => $result]; - } - - /** - * Copy remote file to local storage. - * - * @param array $state - * State array. - * @param \Procrastinator\Result $result - * Job result object. - * - * @return array - * Array with two elements: state and result. - */ - protected function copyRemote(array $state, Result $result): array { - $client = new Client(); - try { - $fout = $this->ensureCreatingForWriting($state['destination']); - $client->get($state['source'], ['sink' => $fout]); - $result->setStatus(Result::DONE); - } - catch (\Exception $e) { - $result->setStatus(Result::ERROR); - $result->setError($e->getMessage()); - } - - $state['total_bytes_copied'] = $state['total_bytes'] = filesize($state['destination']); - return ['state' => $state, 'result' => $result]; - } - - /** - * Ensure the destination file can be created. - * - * @param string $to - * The destination filename. - * - * @return false|resource - * File resource. - */ - private function ensureCreatingForWriting(string $to) { - // Delete destination first to avoid appending if existing. - if (file_exists($to)) { - unlink($to); - } - $fout = fopen($to, "w"); - return $fout; - } - -} diff --git a/modules/datastore/src/Plugin/QueueWorker/ImportJob.php b/modules/datastore/src/Plugin/QueueWorker/ImportJob.php index ca72035886..2625b2a23f 100644 --- a/modules/datastore/src/Plugin/QueueWorker/ImportJob.php +++ b/modules/datastore/src/Plugin/QueueWorker/ImportJob.php @@ -361,7 +361,7 @@ protected function setStorageSchema(array $header) { * Verify headers are unique. * * @param array $header - * List of strings. + * List of strings * * @throws \Exception */ diff --git a/modules/datastore/src/Service.php b/modules/datastore/src/Service.php index dbc8d4747f..df7df8d778 100644 --- a/modules/datastore/src/Service.php +++ b/modules/datastore/src/Service.php @@ -3,7 +3,6 @@ namespace Drupal\datastore; use Drupal\common\DataResource; -use Drupal\datastore\Plugin\QueueWorker\FileFetcherJob; use Drupal\common\Storage\JobStoreFactory; use Procrastinator\Result; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -13,6 +12,7 @@ use Drupal\datastore\Service\ResourceLocalizer; use Drupal\datastore\Service\Factory\ImportFactoryInterface; use Drupal\datastore\Service\Info\ImportInfoList; +use FileFetcher\FileFetcher; /** * Main services for the datastore. @@ -33,13 +33,6 @@ class Service implements ContainerInjectionInterface { */ private $importServiceFactory; - /** - * Import info list service. - * - * @var \Drupal\datastore\Service\Info\ImportInfoList - */ - private ImportInfoList $importInfoList; - /** * Drupal queue. * @@ -211,7 +204,7 @@ public function drop(string $identifier, ?string $version = NULL, bool $local_re if ($local_resource) { $this->resourceLocalizer->remove($identifier, $version); $this->jobStoreFactory - ->getInstance(FileFetcherJob::class) + ->getInstance(FileFetcher::class) ->remove(substr(str_replace('__', '_', $resource_id), 0, -11)); } } diff --git a/modules/datastore/src/Service/Info/ImportInfo.php b/modules/datastore/src/Service/Info/ImportInfo.php index b9c84baf1f..cd81587361 100644 --- a/modules/datastore/src/Service/Info/ImportInfo.php +++ b/modules/datastore/src/Service/Info/ImportInfo.php @@ -6,6 +6,7 @@ use Drupal\common\Storage\JobStoreFactory; use Drupal\datastore\Service\Factory\ImportFactoryInterface; use Drupal\datastore\Service\ResourceLocalizer; +use FileFetcher\FileFetcher; use Procrastinator\Job\Job; /** @@ -37,7 +38,7 @@ class ImportInfo { /** * FileFetcher service. * - * @var \Drupal\datastore\Plugin\QueueWorker\FileFetcherJob + * @var \FileFetcher\FileFetcher */ private $fileFetcher; diff --git a/modules/datastore/src/Service/Info/ImportInfoList.php b/modules/datastore/src/Service/Info/ImportInfoList.php index 6d2e226537..d17aaef238 100644 --- a/modules/datastore/src/Service/Info/ImportInfoList.php +++ b/modules/datastore/src/Service/Info/ImportInfoList.php @@ -2,9 +2,9 @@ namespace Drupal\datastore\Service\Info; -use Drupal\datastore\Plugin\QueueWorker\FileFetcherJob; use Drupal\common\Storage\JobStoreFactory; use Drupal\Core\DependencyInjection\ContainerInjectionInterface; +use FileFetcher\FileFetcher; use Symfony\Component\DependencyInjection\ContainerInterface; /** @@ -58,7 +58,7 @@ public function __construct(JobStoreFactory $jobStoreFactory, ImportInfo $import public function buildList() { $list = []; - $store = $this->jobStoreFactory->getInstance(FileFetcherJob::class); + $store = $this->jobStoreFactory->getInstance(FileFetcher::class); foreach ($store->retrieveAll() as $id) { $pieces = explode("_", $id); diff --git a/modules/datastore/src/Service/ResourceLocalizer.php b/modules/datastore/src/Service/ResourceLocalizer.php index 5f7c97c31e..ea946ecead 100644 --- a/modules/datastore/src/Service/ResourceLocalizer.php +++ b/modules/datastore/src/Service/ResourceLocalizer.php @@ -7,13 +7,14 @@ use Drupal\common\Storage\JobStoreFactory; use Drupal\common\UrlHostTokenResolver; use Drupal\common\Util\DrupalFiles; +use Contracts\FactoryInterface; use Drupal\Core\File\FileSystemInterface; use Drupal\metastore\Exception\AlreadyRegistered; use Drupal\metastore\Reference\Referencer; use Drupal\metastore\ResourceMapper; +use FileFetcher\FileFetcher; use Procrastinator\Result; use Drupal\common\EventDispatcherTrait; -use Drupal\datastore\Plugin\QueueWorker\FileFetcherJob; /** * Resource localizer. @@ -45,6 +46,13 @@ class ResourceLocalizer { */ private $fileMapper; + /** + * DKAN resource file fetcher factory. + * + * @var \Contracts\FactoryInterface + */ + private $fileFetcherFactory; + /** * Drupal files utility service. * @@ -62,8 +70,9 @@ class ResourceLocalizer { /** * Constructor. */ - public function __construct(ResourceMapper $fileMapper, DrupalFiles $drupalFiles, JobStoreFactory $jobStoreFactory) { + public function __construct(ResourceMapper $fileMapper, FactoryInterface $fileFetcherFactory, DrupalFiles $drupalFiles, JobStoreFactory $jobStoreFactory) { $this->fileMapper = $fileMapper; + $this->fileFetcherFactory = $fileFetcherFactory; $this->drupalFiles = $drupalFiles; $this->jobStoreFactory = $jobStoreFactory; } @@ -105,7 +114,7 @@ public function get($identifier, $version = NULL, $perpective = self::LOCAL_FILE /** * Private. */ - private function registerNewPerspectives(DataResource $resource, FileFetcherJob $fileFetcher) { + private function registerNewPerspectives(DataResource $resource, FileFetcher $fileFetcher) { $localFilePath = $fileFetcher->getStateProperty('destination'); $dir = "file://" . $this->drupalFiles->getPublicFilesDirectory(); @@ -170,7 +179,7 @@ private function removeLocalUrl(DataResource $resource) { */ private function removeJob($uuid) { if ($uuid) { - $this->jobStoreFactory->getInstance(FileFetcherJob::class)->remove($uuid); + $this->getJobStoreFactory()->getInstance(FileFetcher::class)->remove($uuid); } } @@ -184,7 +193,7 @@ private function getResourceSource($identifier, $version = NULL): ?DataResource /** * Get FileFetcher. */ - public function getFileFetcher(DataResource $resource): FileFetcherJob { + public function getFileFetcher(DataResource $resource): FileFetcher { $uuid = "{$resource->getIdentifier()}_{$resource->getVersion()}"; $directory = "public://resources/{$uuid}"; $this->getFilesystem()->prepareDirectory($directory, FileSystemInterface::CREATE_DIRECTORY); @@ -192,7 +201,7 @@ public function getFileFetcher(DataResource $resource): FileFetcherJob { 'filePath' => UrlHostTokenResolver::resolveFilePath($resource->getFilePath()), 'temporaryDirectory' => $directory, ]; - return FileFetcherJob::get($uuid, $this->jobStoreFactory->getInstance(FileFetcherJob::class), $config); + return $this->fileFetcherFactory->getInstance($uuid, $config); } /** @@ -202,6 +211,13 @@ private function getFileMapper(): ResourceMapper { return $this->fileMapper; } + /** + * Private. + */ + private function getJobStoreFactory() { + return $this->jobStoreFactory; + } + /** * Get the Drupal filesystem service. * diff --git a/modules/datastore/tests/data/tiny.csv b/modules/datastore/tests/data/tiny.csv deleted file mode 100644 index 9cc135f368..0000000000 --- a/modules/datastore/tests/data/tiny.csv +++ /dev/null @@ -1,2 +0,0 @@ -"hello", "friend" -"goodbye", "enemy" \ No newline at end of file diff --git a/modules/datastore/tests/src/Mock/Container.php b/modules/datastore/tests/src/Mock/Container.php index 5e4da5e4f4..52db31c52f 100644 --- a/modules/datastore/tests/src/Mock/Container.php +++ b/modules/datastore/tests/src/Mock/Container.php @@ -14,6 +14,7 @@ use Drupal\Core\TypedData\TypedDataInterface; use Drupal\datastore\Service\Service; use Drupal\node\NodeInterface; +use FileFetcher\Processor\Local; use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -37,6 +38,7 @@ public function __construct(TestCase $testCase) { $fileFetcherContent = file_get_contents(__DIR__ . '/../../../data/filefetcher.json'); $fileFetcherObject = json_decode($fileFetcherContent); $data = json_decode($fileFetcherObject->result->data); + $data->processor = Local::class; $fileFetcherObject->result->data = json_encode($data); $this->jobStoreData = (object) [ diff --git a/modules/datastore/tests/src/Unit/Plugin/QueueWorker/FileFetcherJobTest.php b/modules/datastore/tests/src/Unit/Plugin/QueueWorker/FileFetcherJobTest.php deleted file mode 100644 index 81ec20baf7..0000000000 --- a/modules/datastore/tests/src/Unit/Plugin/QueueWorker/FileFetcherJobTest.php +++ /dev/null @@ -1,102 +0,0 @@ -getJobstore(); - $config = [ - 'temporaryDirectory' => '/tmp', - 'filePath' => __DIR__ . '/../../../../data/tiny.csv', - ]; - $fetcher = new FileFetcherJob('abc', $jobStore, $config); - $fetcher->run(); - - $state = $fetcher->getState(); - $this->assertEquals( - file_get_contents($state['source']), - file_get_contents($state['destination']) - ); - } - - /** - * Test bad path. - */ - public function testCopyMissingFile() { - $jobStore = $this->getJobstore(); - $config = [ - 'temporaryDirectory' => '/tmp', - 'filePath' => __DIR__ . '/../../../../data/missing.csv', - ]; - $fetcher = new FileFetcherJob('abc', $jobStore, $config); - $fetcher->run(); - - $result = $fetcher->getResult(); - $this->assertStringContainsString("failed to open stream", $result->getError()); - } - - /** - * Test bad url. - */ - public function testCopyBadUrl() { - $jobStore = $this->getJobstore(); - $config = [ - 'temporaryDirectory' => '/tmp', - 'filePath' => 'http://something.fakeurl', - ]; - $fetcher = new FileFetcherJob('abc', $jobStore, $config); - $fetcher->run(); - - $result = $fetcher->getResult(); - $this->assertStringContainsString("Could not resolve host", $result->getError()); - } - - /** - * Test bad destination. - */ - public function testBadDestinationPath() { - $jobStore = $this->getJobstore(); - $config = [ - 'temporaryDirectory' => '/badTempDir', - 'filePath' => __DIR__ . '/../../../../data/tiny.csv', - ]; - $fetcher = new FileFetcherJob('abc', $jobStore, $config); - $fetcher->run(); - - $result = $fetcher->getResult(); - $this->assertStringContainsString("No such file or directory", $result->getError()); - } - - /** - * Test empty config. - */ - public function testInvalidConfig() { - $jobStore = $this->getJobstore(); - $this->expectExceptionMessage("Constructor missing expected config"); - (new FileFetcherJob('abc', $jobStore, [])); - } - - /** - * Get a mock JobStore object. - */ - protected function getJobstore() { - $chain = (new Chain($this)) - ->add(JobStore::class, "setTable", TRUE) - ->add(JobStore::class, "store", "foo"); - - return $chain->getMock(); - } - -} diff --git a/modules/datastore/tests/src/Unit/Service/Info/ImportInfoListTest.php b/modules/datastore/tests/src/Unit/Service/Info/ImportInfoListTest.php index 4c1b9756e8..7f4805ef0a 100644 --- a/modules/datastore/tests/src/Unit/Service/Info/ImportInfoListTest.php +++ b/modules/datastore/tests/src/Unit/Service/Info/ImportInfoListTest.php @@ -7,7 +7,7 @@ use Drupal\common\Storage\JobStoreFactory; use Drupal\datastore\Service\Info\ImportInfo; use Drupal\datastore\Service\Info\ImportInfoList; -use Drupal\datastore\Plugin\QueueWorker\FileFetcherJob; +use FileFetcher\FileFetcher; use MockChain\Chain; use MockChain\Options; use PHPUnit\Framework\TestCase; @@ -17,7 +17,7 @@ class ImportInfoListTest extends TestCase { public function test() { - $ff = FileFetcherJob::hydrate('{}'); + $ff = FileFetcher::hydrate('{}'); $result = Result::hydrate('{"status":"error","data":"","error":"File import error"}'); diff --git a/modules/datastore/tests/src/Unit/Service/ResourceLocalizerTest.php b/modules/datastore/tests/src/Unit/Service/ResourceLocalizerTest.php index 8ddbc220c2..ff5ad9d269 100644 --- a/modules/datastore/tests/src/Unit/Service/ResourceLocalizerTest.php +++ b/modules/datastore/tests/src/Unit/Service/ResourceLocalizerTest.php @@ -12,7 +12,7 @@ use Drupal\Core\StreamWrapper\StreamWrapperManager; use Drupal\datastore\Service\ResourceLocalizer; use Drupal\metastore\ResourceMapper; -use Drupal\datastore\Plugin\QueueWorker\FileFetcherJob; +use FileFetcher\FileFetcher; use MockChain\Chain; use MockChain\Options; use PHPUnit\Framework\TestCase; @@ -40,6 +40,7 @@ public function testNoResourceFound() { $service = new ResourceLocalizer( $this->getFileMapperChain()->getMock(), + $this->getFileFetcherFactoryChain()->getMock(), $this->getDrupalFilesChain()->getMock(), $this->getJobStoreFactoryChain()->getMock() ); @@ -67,8 +68,13 @@ private function doTestResourceLocalizerRemove(string $file_path): void { ->add(ResourceMapper::class, 'get', $resource) ->getMock(); + $fileFetcher = $this->getFileFetcherFactoryChain() + ->add(FileFetcher::class, 'getStateProperty', $file_path) + ->getMock(); + $service = new ResourceLocalizer( $fileMapper, + $fileFetcher, $this->getDrupalFilesChain()->getMock(), $this->getJobStoreFactoryChain()->getMock() ); @@ -117,6 +123,16 @@ private function getFileMapperChain() { ->add(ResourceMapper::class, 'remove', NULL); } + /** + * + */ + private function getFileFetcherFactoryChain() { + return (new Chain($this)) + ->add(JobStoreFactory::class, 'getInstance', FileFetcher::class) + ->add(FileFetcher::class, 'getResult', Result::class) + ->add(Result::class, 'getStatus', Result::DONE); + } + /** * */ @@ -135,11 +151,7 @@ private function getDrupalFilesChain() { private function getJobStoreFactoryChain() { return (new Chain($this)) ->add(JobStoreFactory::class, 'getInstance', JobStore::class) - ->add(JobStore::class, 'remove', NULL) - ->add(JobStore::class, 'tableExist', FALSE) - ->add(JobStore::class, 'setTable', TRUE) - ->add(JobStore::class, 'retrieve', []) - ->add(JobStore::class, 'store', "abc"); + ->add(JobStore::class, 'remove', NULL); } /** diff --git a/modules/datastore/tests/src/Unit/ServiceTest.php b/modules/datastore/tests/src/Unit/ServiceTest.php index 7b13d018c6..40ab3e67be 100644 --- a/modules/datastore/tests/src/Unit/ServiceTest.php +++ b/modules/datastore/tests/src/Unit/ServiceTest.php @@ -15,10 +15,11 @@ use Drupal\datastore\Service\ResourceLocalizer; use Drupal\datastore\Storage\DatabaseTable; use Drupal\metastore\ResourceMapper; -use Drupal\datastore\Plugin\QueueWorker\FileFetcherJob; +use FileFetcher\FileFetcher; use MockChain\Chain; use MockChain\Options; use PHPUnit\Framework\TestCase; +use Procrastinator\Job\AbstractPersistentJob; use Procrastinator\Result; use Symfony\Component\DependencyInjection\Container; use TypeError; @@ -37,7 +38,7 @@ public function testImport() { $chain = $this->getContainerChainForService('dkan.datastore.service') ->add(ResourceLocalizer::class, 'get', $resource) ->add(ResourceLocalizer::class, 'getResult', Result::class) - ->add(FileFetcherJob::class, 'run', Result::class) + ->add(FileFetcher::class, 'run', Result::class) ->add(ResourceMapper::class, 'get', $resource) ->add(ImportServiceFactory::class, "getInstance", ImportService::class) ->add(ImportService::class, "import", NULL) diff --git a/modules/datastore/tests/src/Unit/Storage/JobStoreTest.php b/modules/datastore/tests/src/Unit/Storage/JobStoreTest.php index 9703976cf0..d0b02d4699 100644 --- a/modules/datastore/tests/src/Unit/Storage/JobStoreTest.php +++ b/modules/datastore/tests/src/Unit/Storage/JobStoreTest.php @@ -12,7 +12,7 @@ use MockChain\Chain; use MockChain\Sequence; use Drupal\common\Storage\JobStore; -use Drupal\datastore\Plugin\QueueWorker\FileFetcherJob; +use FileFetcher\FileFetcher; use PHPUnit\Framework\TestCase; /** @@ -29,7 +29,7 @@ public function testConstruction() { ->add(Connection::class, "schema", Schema::class) ->add(Schema::class, "tableExists", FALSE); - $jobStore = new JobStore(FileFetcherJob::class, $chain->getMock()); + $jobStore = new JobStore(FileFetcher::class, $chain->getMock()); $this->assertTrue(is_object($jobStore)); } @@ -58,8 +58,8 @@ public function testRetrieve() { ->add(Connection::class, 'query', StatementWrapper::class) ->add(StatementWrapper::class, 'fetchAll', $fieldInfo); - $jobStore = new JobStore(FileFetcherJob::class, $chain->getMock()); - $this->assertEquals($job_data, $jobStore->retrieve("1", FileFetcherJob::class)); + $jobStore = new JobStore(FileFetcher::class, $chain->getMock()); + $this->assertEquals($job_data, $jobStore->retrieve("1", FileFetcher::class)); } /** @@ -89,7 +89,7 @@ public function testRetrieveAll() { ->add(Connection::class, 'query', StatementWrapper::class) ->add(StatementWrapper::class, 'fetchAll', $sequence); - $jobStore = new JobStore(FileFetcherJob::class, $chain->getMock()); + $jobStore = new JobStore(FileFetcher::class, $chain->getMock()); $this->assertTrue(is_array($jobStore->retrieveAll())); } @@ -126,7 +126,7 @@ public function testStore() { ->add(StatementWrapper::class, 'fetchAll', $fieldInfo) ->getMock(); - $jobStore = new JobStore(FileFetcherJob::class, $connection); + $jobStore = new JobStore(FileFetcher::class, $connection); $this->assertEquals("1", $jobStore->store(json_encode($jobObject), "1")); } @@ -150,16 +150,16 @@ public function testRemove() { ->add(StatementWrapper::class, 'fetchAll', $fieldInfo) ->getMock(); - $jobStore = new JobStore(FileFetcherJob::class, $connection); + $jobStore = new JobStore(FileFetcher::class, $connection); - $this->assertEquals("", $jobStore->remove("1", FileFetcherJob::class)); + $this->assertEquals("", $jobStore->remove("1", FileFetcher::class)); } /** * Private. */ private function getFileFetcher() { - return FileFetcherJob::get("1", new Memory(), ["filePath" => "file://" . __DIR__ . "/../../data/countries.csv"]); + return FileFetcher::get("1", new Memory(), ["filePath" => "file://" . __DIR__ . "/../../data/countries.csv"]); } } diff --git a/modules/sample_content/sample_content.json b/modules/sample_content/sample_content.json index e3f1402586..39137987dc 100644 --- a/modules/sample_content/sample_content.json +++ b/modules/sample_content/sample_content.json @@ -16,7 +16,7 @@ "distribution": [ { "@type": "dcat:Distribution", - "downloadURL": "file:\/\//var/www/html/docroot/modules/contrib/dkan/modules/sample_content/files\/Bike_Lane.csv", + "downloadURL": "file:\/\//var/www/docroot/modules/contrib/dkan/modules/sample_content/files\/Bike_Lane.csv", "mediaType": "text\/csv", "format": "csv", "title": "Florida Bike Lanes" @@ -49,21 +49,21 @@ "distribution":[ { "@type":"dcat:Distribution", - "downloadURL":"file:\/\//var/www/html/docroot/modules/contrib/dkan/modules/sample_content/files\/pittsmva2016data.zip", + "downloadURL":"file:\/\//var/www/docroot/modules/contrib/dkan/modules/sample_content/files\/pittsmva2016data.zip", "mediaType":"application\/zip", "format":"zip", "title":"MVA Geographic Data 2016" }, { "@type":"dcat:Distribution", - "downloadURL":"file:\/\//var/www/html/docroot/modules/contrib/dkan/modules/sample_content/files\/2016mvadatadictionary.pdf", + "downloadURL":"file:\/\//var/www/docroot/modules/contrib/dkan/modules/sample_content/files\/2016mvadatadictionary.pdf", "mediaType":"application\/pdf", "format":"pdf", "title":"2016 MVA Data Dictionary" }, { "@type":"dcat:Distribution", - "downloadURL":"file:\/\//var/www/html/docroot/modules/contrib/dkan/modules/sample_content/files\/IMD-MAPS.zip", + "downloadURL":"file:\/\//var/www/docroot/modules/contrib/dkan/modules/sample_content/files\/IMD-MAPS.zip", "mediaType":"application\/zip", "format":"zip", "title":"Map files" @@ -94,7 +94,7 @@ "distribution":[ { "@type":"dcat:Distribution", - "downloadURL":"file:\/\//var/www/html/docroot/modules/contrib/dkan/modules/sample_content/files\/ViolentCrimeRates.csv", + "downloadURL":"file:\/\//var/www/docroot/modules/contrib/dkan/modules/sample_content/files\/ViolentCrimeRates.csv", "mediaType":"text\/csv", "format":"csv", "description":"Includes information on the frequency of violent crimes in general, murder and non-negligent manslaughter, aggravated assault, rape and robbery. All crime rates are per 100,000 people.", @@ -124,7 +124,7 @@ "distribution":[ { "@type":"dcat:Distribution", - "downloadURL":"file:\/\//var/www/html/docroot/modules/contrib/dkan/modules/sample_content/files\/Asthma-Prevalence-Map-2017.csv", + "downloadURL":"file:\/\//var/www/docroot/modules/contrib/dkan/modules/sample_content/files\/Asthma-Prevalence-Map-2017.csv", "mediaType":"text\/csv", "format":"csv", "description":"Mock data for demonstration purposes.", @@ -156,7 +156,7 @@ "distribution":[ { "@type":"dcat:Distribution", - "downloadURL":"file:\/\//var/www/html/docroot/modules/contrib/dkan/modules/sample_content/files\/CDCSmokingRates.csv", + "downloadURL":"file:\/\//var/www/docroot/modules/contrib/dkan/modules/sample_content/files\/CDCSmokingRates.csv", "mediaType":"text\/csv", "format":"csv", "description":"This dataset highlights critical trends in adult total and per capita consumption of both combustible (cigarettes, little cigars, small cigars, pipe tobacco, roll-your-own tobacco) tobacco products and smokeless (chewing tobacco and snuff) tobacco.", @@ -188,7 +188,7 @@ "distribution":[ { "@type":"dcat:Distribution", - "downloadURL":"file:\/\//var/www/html/docroot/modules/contrib/dkan/modules/sample_content/files\/demo_varicelladeaths_1991_2007.csv", + "downloadURL":"file:\/\//var/www/docroot/modules/contrib/dkan/modules/sample_content/files\/demo_varicelladeaths_1991_2007.csv", "mediaType":"text\/csv", "format":"csv", "description":"Mock data for demonstration purposes.", @@ -196,7 +196,7 @@ }, { "@type":"dcat:Distribution", - "downloadURL":"file:\/\//var/www/html/docroot/modules/contrib/dkan/modules/sample_content/files\/demo_varicelladeaths_1974_1990.csv", + "downloadURL":"file:\/\//var/www/docroot/modules/contrib/dkan/modules/sample_content/files\/demo_varicelladeaths_1974_1990.csv", "mediaType":"text\/csv", "format":"csv", "description":"Mock data for demonstration purposes.", @@ -228,7 +228,7 @@ "distribution":[ { "@type":"dcat:Distribution", - "downloadURL": "file:\/\//var/www/html/docroot/modules/contrib/dkan/modules/sample_content/files\/polling-places.csv", + "downloadURL": "file:\/\//var/www/docroot/modules/contrib/dkan/modules/sample_content/files\/polling-places.csv", "mediaType":"text\/csv", "format":"csv", "description":"List of polling places in Madison, WI.", @@ -259,7 +259,7 @@ "distribution":[ { "@type":"dcat:Distribution", - "downloadURL":"file:\/\//var/www/html/docroot/modules/contrib/dkan/modules/sample_content/files\/us_foreclosures_jan_2012_by_state.csv", + "downloadURL":"file:\/\//var/www/docroot/modules/contrib/dkan/modules/sample_content/files\/us_foreclosures_jan_2012_by_state.csv", "mediaType":"text\/csv", "format":"csv", "description":"US National Foreclosure Statistics - By State - January 2012", @@ -287,7 +287,7 @@ "distribution":[ { "@type":"dcat:Distribution", - "downloadURL":"file:\/\//var/www/html/docroot/modules/contrib/dkan/modules/sample_content/files\/data.csv", + "downloadURL":"file:\/\//var/www/docroot/modules/contrib/dkan/modules/sample_content/files\/data.csv", "mediaType":"text\/csv", "format":"csv", "title":"Table of Gold Prices" @@ -318,7 +318,7 @@ "distribution":[ { "@type":"dcat:Distribution", - "downloadURL":"file:\/\//var/www/html/docroot/modules/contrib/dkan/modules/sample_content/files\/district_centerpoints.csv", + "downloadURL":"file:\/\//var/www/docroot/modules/contrib/dkan/modules/sample_content/files\/district_centerpoints.csv", "mediaType":"text\/csv", "format":"csv", "title":"District Names" From 60b765aec17207001b0d2c9ed5d1aed6e7525886 Mon Sep 17 00:00:00 2001 From: Dan Feder Date: Wed, 21 Dec 2022 13:47:07 -0500 Subject: [PATCH 2/4] CodeClimate fix --- modules/datastore/src/Plugin/QueueWorker/ImportJob.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/datastore/src/Plugin/QueueWorker/ImportJob.php b/modules/datastore/src/Plugin/QueueWorker/ImportJob.php index 2625b2a23f..ca72035886 100644 --- a/modules/datastore/src/Plugin/QueueWorker/ImportJob.php +++ b/modules/datastore/src/Plugin/QueueWorker/ImportJob.php @@ -361,7 +361,7 @@ protected function setStorageSchema(array $header) { * Verify headers are unique. * * @param array $header - * List of strings + * List of strings. * * @throws \Exception */ From 5137db583943422d6fb0306b4b34f500e1e20a26 Mon Sep 17 00:00:00 2001 From: Dan Feder Date: Wed, 21 Dec 2022 13:47:16 -0500 Subject: [PATCH 3/4] Use new filefetcher --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 926bd0defe..2cf2d19f20 100644 --- a/composer.json +++ b/composer.json @@ -18,7 +18,7 @@ "fmizzell/maquina": "^1.1.0", "getdkan/contracts": "^1.0.0", "getdkan/csv-parser": "^1.2.3", - "getdkan/file-fetcher" : "^4.1.0", + "getdkan/file-fetcher" : "dev-guzzle-remote", "getdkan/harvest": "^1.0.0", "getdkan/json-schema-provider": "^0.1.2", "getdkan/locker": "^1.1.0", From c0facae3fa29b0d7298f6cfab4a1726bca58e10e Mon Sep 17 00:00:00 2001 From: Dan Feder Date: Fri, 17 Feb 2023 13:28:28 -0500 Subject: [PATCH 4/4] Update composer.json --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 2cf2d19f20..d67a5d2849 100644 --- a/composer.json +++ b/composer.json @@ -18,7 +18,7 @@ "fmizzell/maquina": "^1.1.0", "getdkan/contracts": "^1.0.0", "getdkan/csv-parser": "^1.2.3", - "getdkan/file-fetcher" : "dev-guzzle-remote", + "getdkan/file-fetcher" : "4.2.0", "getdkan/harvest": "^1.0.0", "getdkan/json-schema-provider": "^0.1.2", "getdkan/locker": "^1.1.0",