diff --git a/.circleci/config.yml b/.circleci/config.yml index f9e2290d23..30f2f6a9e9 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -29,7 +29,7 @@ commands: default: '8.2' type: string addon_branch: - description: 'Repo branch name for the dkan-ddev-addon you want to test against.' + description: 'Repo branch name for the DKAN DDev add-on you want to test against.' default: 'main' type: string dkan_recommended_branch: diff --git a/modules/common/src/Util/DrupalFiles.php b/modules/common/src/Util/DrupalFiles.php index eda67b6a0a..12ed2a733a 100644 --- a/modules/common/src/Util/DrupalFiles.php +++ b/modules/common/src/Util/DrupalFiles.php @@ -53,7 +53,10 @@ public function __construct(FileSystemInterface $filesystem, StreamWrapperManage } /** - * Getter. + * Get the Drupal file_system service. + * + * @returns FileSystemInterface + * The file_system service. */ public function getFilesystem(): FileSystemInterface { return $this->filesystem; @@ -84,6 +87,7 @@ public function retrieveFile($url, $destination) { throw new \Exception("Only moving files to Drupal's public directory (public://) is supported"); } + // Handle file:// URIs. if (substr_count($url, "file://") > 0) { $src = str_replace("file://", "", $url); @@ -93,15 +97,17 @@ public function retrieveFile($url, $destination) { return $this->fileCreateUrl("{$destination}/{$filename}"); } - else { - return system_retrieve_file($url, $destination, FALSE, FileSystemInterface::EXISTS_REPLACE); - } + // Handle http(s):// URIs. + return system_retrieve_file($url, $destination, FALSE, FileSystemInterface::EXISTS_REPLACE); } /** - * Given a URI like public:// retrieve the URL. + * Given a URI like public://, retrieve the http URL. + * + * @returns string + * The URL. */ - public function fileCreateUrl($uri) { + public function fileCreateUrl($uri) : string { if (substr_count($uri, 'http') > 0) { return $uri; } diff --git a/modules/harvest/src/Transform/ResourceImporter.php b/modules/harvest/src/Transform/ResourceImporter.php index 0f2e78ceae..9f31a56a7b 100644 --- a/modules/harvest/src/Transform/ResourceImporter.php +++ b/modules/harvest/src/Transform/ResourceImporter.php @@ -3,21 +3,39 @@ namespace Drupal\harvest\Transform; use Drupal\Core\File\FileSystemInterface; +use Drupal\Core\File\FileUrlGeneratorInterface; +use Drupal\common\Util\DrupalFiles; use Harvest\ETL\Transform\Transform; /** * Moves local files to public:// and alters the downloadUrl field. * - * @codeCoverageIgnore + * Used by the sample_content harvest. + * + * @see modules/sample_content/harvest_plan.json */ class ResourceImporter extends Transform { /** - * Drupal files. + * DKAN's Drupal files service. * * @var \Drupal\common\Util\DrupalFiles */ - private $drupalFiles; + private DrupalFiles $drupalFiles; + + /** + * File URL generator service. + * + * @var \Drupal\Core\File\FileUrlGeneratorInterface + */ + private FileUrlGeneratorInterface $fileUrlGenerator; + + /** + * Drupal's file system service. + * + * @var \Drupal\Core\File\FileSystemInterface + */ + private FileSystemInterface $fileSystem; /** * Constructor. @@ -25,6 +43,8 @@ class ResourceImporter extends Transform { public function __construct($harvest_plan) { parent::__construct($harvest_plan); $this->drupalFiles = \Drupal::service('dkan.common.drupal_files'); + $this->fileUrlGenerator = \Drupal::service('file_url_generator'); + $this->fileSystem = \Drupal::service('file_system'); } /** @@ -96,10 +116,6 @@ protected function updateDownloadUrl($dataset, $dist) { /** * Pulls down external file and saves it locally. * - * If this method is called when PHP is running on the CLI (e.g. via drush), - * `$settings['file_public_base_url']` must be configured in `settings.php`, - * otherwise 'default' will be used as the hostname in the new URL. - * * @param string $url * External file URL. * @param string $dataset_id @@ -108,12 +124,10 @@ protected function updateDownloadUrl($dataset, $dist) { * @return string|bool * The URL for the newly created file, or FALSE if failure occurs. */ - public function saveFile($url, $dataset_id) { - + public function saveFile(string $url, string $dataset_id) { $targetDir = 'public://distribution/' . $dataset_id; - $this->drupalFiles - ->getFilesystem() + $this->fileSystem ->prepareDirectory($targetDir, FileSystemInterface::CREATE_DIRECTORY); // Abort if file can't be saved locally. @@ -122,11 +136,9 @@ public function saveFile($url, $dataset_id) { } if (is_object($path)) { - return \Drupal::service('file_url_generator')->generateAbsoluteString($path->uri->value); - } - else { - return $this->drupalFiles->fileCreateUrl($path); + return $this->fileUrlGenerator->generateAbsoluteString($path->uri->value); } + return $this->drupalFiles->fileCreateUrl($path); } } diff --git a/modules/harvest/tests/src/Functional/Transform/ResourceImporterTest.php b/modules/harvest/tests/src/Functional/Transform/ResourceImporterTest.php new file mode 100644 index 0000000000..5a8d38a17a --- /dev/null +++ b/modules/harvest/tests/src/Functional/Transform/ResourceImporterTest.php @@ -0,0 +1,47 @@ +container->get('extension.list.module') + ->getPath('sample_content'); + + $importer = new ResourceImporter('harvest_plan_id'); + + $this->assertStringContainsString( + 'distribution/dataset/Bike_Lane.csv', + $importer->saveFile( + 'file://' . $sample_content_module_path . '/files/Bike_Lane.csv', + 'dataset' + ) + ); + } + +} diff --git a/modules/harvest/tests/src/Kernel/Transform/ResourceImporterTest.php b/modules/harvest/tests/src/Kernel/Transform/ResourceImporterTest.php new file mode 100644 index 0000000000..432057e42d --- /dev/null +++ b/modules/harvest/tests/src/Kernel/Transform/ResourceImporterTest.php @@ -0,0 +1,101 @@ +assertIsObject($result = $importer->run($dataset)); + $this->assertEquals($dataset, $result); + + // Calling with a distribution with no download url should result in the + // same object being returned. + $dataset = (object) [ + 'distribution' => [ + (object) [ + 'title' => 'my title', + ], + ], + ]; + $this->assertIsObject($result = $importer->run($dataset)); + $this->assertEquals($dataset, $result); + } + + /** + * @covers ::updateDownloadUrl + */ + public function testUpdateDownloadUrl() { + // Mock saveFile so we don't actually have to worry about the file system. + $importer = $this->getMockBuilder(ResourceImporter::class) + ->setConstructorArgs(['harvest_plan_id']) + ->onlyMethods(['saveFile']) + ->getMock(); + $importer->expects($this->any()) + ->method('saveFile') + // Adds '_saved' to whatever URL was passed in. + ->willReturnCallback(function ($url, $dataset_id): string { + return $url . '_saved'; + }); + + // Prepare a dataset with a downloadURL. + $dataset = (object) [ + 'identifier' => 'identifier', + 'distribution' => [ + (object) [ + 'downloadURL' => 'my_url', + 'title' => 'my title', + ], + ], + ]; + $this->assertIsObject($result = $importer->run($dataset)); + $this->assertEquals( + 'my_url_saved', + $result->distribution[0]->downloadURL ?? 'nope' + ); + } + + /** + * @covers ::saveFile + */ + public function testSaveFile() { + // When DrupalFiles::retrieveFile fails, saveFile will return FALSE. + $drupal_files = $this->getMockBuilder(DrupalFiles::class) + ->setConstructorArgs([ + $this->container->get('file_system'), + $this->container->get('stream_wrapper_manager'), + ]) + ->onlyMethods(['retrieveFile']) + ->getMock(); + $drupal_files->expects($this->once()) + ->method('retrieveFile') + ->willReturn(FALSE); + + $this->container->set('dkan.common.drupal_files', $drupal_files); + + $importer = new ResourceImporter('harvest_plan_id'); + $this->assertFalse($importer->saveFile('any_url', 'any_dataset')); + } + +} diff --git a/modules/metastore/metastore.info.yml b/modules/metastore/metastore.info.yml index ea7d02d536..61ad9d380d 100644 --- a/modules/metastore/metastore.info.yml +++ b/modules/metastore/metastore.info.yml @@ -7,3 +7,4 @@ dependencies: - dkan:common - drupal:basic_auth - drupal:content_moderation + - drupal:node diff --git a/modules/sample_content/src/SampleContentService.php b/modules/sample_content/src/SampleContentService.php index cff7d903e2..1532ec5191 100644 --- a/modules/sample_content/src/SampleContentService.php +++ b/modules/sample_content/src/SampleContentService.php @@ -21,7 +21,7 @@ class SampleContentService { private string $modulePath; /** - * Constructor for the Sample Content commands. + * Constructor for the Sample Content service. * * @param \Drupal\Core\Extension\ModuleExtensionList $moduleExtensionList * Extension list. diff --git a/modules/sample_content/tests/src/Functional/SampleContentCommandsTest.php b/modules/sample_content/tests/src/Functional/SampleContentCommandsTest.php index a21904e73f..85052415db 100644 --- a/modules/sample_content/tests/src/Functional/SampleContentCommandsTest.php +++ b/modules/sample_content/tests/src/Functional/SampleContentCommandsTest.php @@ -3,7 +3,10 @@ namespace Drupal\Tests\sample_content\Functional; use Drupal\Tests\BrowserTestBase; +use Drupal\user\Entity\User; use Drush\TestTraits\DrushTestTrait; +use GuzzleHttp\Client; +use GuzzleHttp\RequestOptions; /** * @coversDefaultClass \Drupal\sample_content\Drush @@ -21,20 +24,56 @@ class SampleContentCommandsTest extends BrowserTestBase { protected $strictConfigSchema = FALSE; protected static $modules = [ + 'datastore', 'node', 'sample_content', ]; + /** + * Get a Guzzle Client object ready for sending HTTP requests. + * + * @param \Drupal\user\Entity\User|null $authUser + * (Optional) A user object to use for authentication. + * @param bool $http_errors + * (Optional) Whether 4xx or 5xx response codes should throw an exception. + * Defaults to FALSE. + * + * @return \GuzzleHttp\Client + * Client ready for HTTP requests. + * + * @todo Move this to a trait or base class. + */ + protected function getApiClient(?User $authUser = NULL, $http_errors = FALSE): Client { + $options = [ + 'base_uri' => $this->baseUrl, + RequestOptions::HTTP_ERRORS => $http_errors, + ]; + if ($authUser) { + $options[RequestOptions::AUTH] = [$authUser->getAccountName(), $authUser->pass_raw]; + } + return $this->container->get('http_client_factory')->fromOptions($options); + } + public function test() { + // No errors thrown on --help. + foreach ([ + 'dkan:sample-content:create', + 'dkan:sample-content:remove', + ] as $command) { + $this->drush($command . ' --help'); + $this->assertEmpty( + $this->getSimplifiedErrorOutput() + ); + } + $harvest_plan_name = 'sample_content'; + /** @var \Drupal\harvest\HarvestService $harvest_service */ + $harvest_service = $this->container->get('dkan.harvest.service'); // Run the create command. $this->drush('dkan:sample-content:create'); $output = $this->getOutput(); - /** @var \Drupal\harvest\HarvestService $harvest_service */ - $harvest_service = $this->container->get('dkan.harvest.service'); - // Start asserting. foreach ([ 'run_id', @@ -59,9 +98,55 @@ public function test() { $this->assertCount(10, $run_info['status']['extracted_items_ids'] ?? []); $this->assertEmpty($run_info['error'] ?? []); + // What does the RESTful API say? + $response = $this->getApiClient()->get('/api/1/metastore/schemas/dataset/items'); + $this->assertEquals(200, $response->getStatusCode()); + $this->assertCount(10, json_decode($response->getBody()->getContents())); + + // Do the import. + $this->drush('queue:run localize_import'); + $this->assertStringContainsString( + 'Processed 10 items from the localize_import queue', + $this->getSimplifiedErrorOutput() + ); + $this->drush('queue:run datastore_import'); + $this->assertStringContainsString( + 'Processed 10 items from the datastore_import queue', + $this->getSimplifiedErrorOutput() + ); + + // What does the RESTful API say? + $response = $this->getApiClient()->get('/api/1/metastore/schemas/dataset/items'); + $this->assertEquals(200, $response->getStatusCode()); + $this->assertCount(10, json_decode($response->getBody()->getContents())); + + // Find the bike lanes dataset. + $identifier = 'cedcd327-4e5d-43f9-8eb1-c11850fa7c55'; + $response = $this->getApiClient()->get('/api/1/metastore/schemas/dataset/items/' . $identifier); + $this->assertEquals(200, $response->getStatusCode()); + $this->assertIsObject($payload = json_decode($response->getBody()->getContents())); + $this->assertEquals($identifier, $payload->identifier ?? 'nope'); + + // There is a download URL. + $this->assertNotEmpty( + $download_url = $payload->distribution[0]->downloadURL ?? '' + ); + // Ensure the server is not 'default'. + $this->assertStringNotContainsString('default', parse_url($download_url, PHP_URL_HOST)); + + // We can reach the download URL. Roll our own Guzzle client since we don't + // want a base URL option. + /** @var \GuzzleHttp\Client $client */ + $client = $this->container->get('http_client_factory')->fromOptions([ + RequestOptions::HTTP_ERRORS => FALSE, + ]); + // HEAD request since we don't want to actually transfer the file right + // now. + $this->assertNotEmpty($response = $client->head($download_url)); + $this->assertEquals(200, $response->getStatusCode()); + // Run the remove command. $this->drush('dkan:sample-content:remove'); - // Logged output counts as an error, even if it's not an error. $output = $this->getErrorOutput(); // Assert the output. @@ -71,6 +156,10 @@ public function test() { ] as $expected) { $this->assertStringContainsString($expected, $output); } + + $response = $this->getApiClient()->get('/api/1/metastore/schemas/dataset/items'); + $this->assertEquals(200, $response->getStatusCode()); + $this->assertCount(0, json_decode($response->getBody()->getContents())); } }