Use same sanitization when comparing properties (#4233)

modules/common/src/Storage/SelectFactory.php

@@ -104,8 +104,10 @@ private function setQueryProperties(Query $query) {
   private function addDateExpressions($db_query, $fields, $meta_data) {
     foreach ($meta_data as $definition) {
       // Confirm definition name is in the fields list.
-      if ($fields[$definition['name']]['field'] && $definition['type'] == 'date') {
-        $db_query->addExpression("DATE_FORMAT(" . $definition['name'] . ", '" . $definition['format'] . "')", $definition['name']);
+      $name = $this->dbQuery->escapeField($definition['name']);
+      $sanitizedName = $fields[$name]['field'];
+      if ($sanitizedName && $definition['type'] == 'date') {
+        $db_query->addExpression("DATE_FORMAT(" . $sanitizedName . ", '" . $definition['format'] . "')", $sanitizedName);
       }
     }
   }

modules/common/tests/src/Traits/GetDataTrait.php

@@ -22,11 +22,14 @@ private function getDownloadUrl(string $filename) {
    *   Array of resource files URLs for this dataset.
    * @param bool $localFiles
    *   Whether the resource files are local.
+   * @param string $describedBy
+   *   (Optional) URI for describedBy for all the download URLs. describedByType
+   *   will be set to 'application/vnd.tableschema+json' if present.
    *
    * @return string|false
    *   Json encoded string of this dataset's metadata, or FALSE if error.
    */
-  private function getDataset(string $identifier, string $title, array $downloadUrls, bool $localFiles = FALSE) {
+  private function getDataset(string $identifier, string $title, array $downloadUrls, bool $localFiles = FALSE, string $describedBy = NULL) {
 
     $data = new \stdClass();
     $data->title = $title;
@@ -42,6 +45,10 @@ private function getDataset(string $identifier, string $title, array $downloadUr
       $distribution->title = "Distribution #{$key} for {$identifier}";
       $distribution->downloadURL = $localFiles ? $downloadUrl : $this->getDownloadUrl($downloadUrl);
       $distribution->mediaType = "text/csv";
+      if ($describedBy) {
+        $distribution->describedBy = $describedBy;
+        $distribution->describedByType = 'application/vnd.tableschema+json';
+      }
 
       $data->distribution[] = $distribution;
     }

modules/datastore/modules/datastore_mysql_import/tests/src/Functional/Controller/MySqlQueryDownloadControllerTest.php

@@ -0,0 +1,27 @@
+<?php
+
+namespace Drupal\Tests\datastore_mysql_import\Functional\Controller;
+
+use Drupal\Tests\datastore\Functional\Controller\QueryDownloadControllerTest;
+
+/**
+ * Test streaming CSV downloads with data dictionaries.
+ *
+ * This is the same test as
+ * \Drupal\Tests\datastore\Functional\Controller\QueryDownloadControllerTest,
+ * but using the mysql importer.
+ *
+ * @group dkan
+ * @group datastore_mysql_import
+ * @group functional
+ * @group btb
+ *
+ * @see \Drupal\Tests\datastore\Functional\Controller\QueryDownloadControllerTest
+ */
+class MySqlQueryDownloadControllerTest extends QueryDownloadControllerTest {
+
+  protected static $modules = [
+    'datastore_mysql_import',
+  ];
+
+}

modules/datastore/tests/src/Functional/Controller/QueryDownloadControllerTest.php

@@ -3,6 +3,7 @@
 namespace Drupal\Tests\datastore\Functional\Controller;
 
 use Drupal\Core\File\FileSystemInterface;
+use Drupal\metastore\DataDictionary\DataDictionaryDiscovery;
 use Drupal\Tests\BrowserTestBase;
 use Drupal\Tests\common\Traits\GetDataTrait;
 use Drupal\Tests\common\Traits\QueueRunnerTrait;
@@ -34,14 +35,6 @@ class QueryDownloadControllerTest extends BrowserTestBase {
    */
   protected const TEST_DATA_PATH = __DIR__ . '/../../../data/';
 
-  /**
-   * Resource file name.
-   *
-   * @var string
-   */
-  protected const RESOURCE_FILE = 'longcolumn.csv';
-
-
   protected static $modules = [
     'datastore',
     'node',
@@ -53,21 +46,14 @@ class QueryDownloadControllerTest extends BrowserTestBase {
    * Test application of data dictionary schema to CSV generated for download.
    */
   public function testDownloadWithMachineName() {
+    $resourceFile = 'longcolumn.csv';
     // Dependencies.
     $uuid = $this->container->get('uuid');
     /** @var \Drupal\metastore\ValidMetadataFactory $validMetadataFactory */
     $validMetadataFactory = $this->container->get('dkan.metastore.valid_metadata');
     /** @var \Drupal\metastore\MetastoreService $metastoreService */
     $metastoreService = $this->container->get('dkan.metastore.service');
-    // Copy resource file to uploads directory.
-    /** @var \Drupal\Core\File\FileSystemInterface $file_system */
-    $file_system = $this->container->get('file_system');
-    $upload_path = $file_system->realpath(self::UPLOAD_LOCATION);
-    $file_system->prepareDirectory($upload_path, FileSystemInterface::CREATE_DIRECTORY);
-    $file_system->copy(self::TEST_DATA_PATH . self::RESOURCE_FILE, $upload_path, FileSystemInterface::EXISTS_REPLACE);
-    $resourceUrl = $this->container->get('stream_wrapper_manager')
-      ->getViaUri(self::UPLOAD_LOCATION . self::RESOURCE_FILE)
-      ->getExternalUrl();
+    $resourceUrl = $this->setUpResourceFile($resourceFile);
 
     // Set up dataset.
     $dataset_id = $uuid->generate();
@@ -139,4 +125,162 @@ public function testDownloadWithMachineName() {
     );
   }
 
+  /**
+   * Test application of data dictionary schema to CSV generated for download.
+   */
+  public function testDownloadWithDataDictionary() {
+    // Set per-reference data-dictinary in metastore config.
+    $this->config('metastore.settings')
+      ->set('data_dictionary_mode', DataDictionaryDiscovery::MODE_REFERENCE)
+      ->save();
+    $this->assertEquals(
+      DataDictionaryDiscovery::MODE_REFERENCE,
+      $this->config('metastore.settings')->get('data_dictionary_mode')
+    );
+
+    $resourceFile = 'data-dict.csv';
+
+    // Dependencies.
+    $uuid = $this->container->get('uuid');
+    /** @var \Drupal\metastore\ValidMetadataFactory $validMetadataFactory */
+    $validMetadataFactory = $this->container->get('dkan.metastore.valid_metadata');
+    /** @var \Drupal\metastore\MetastoreService $metastore */
+    $metastore = $this->container->get('dkan.metastore.service');
+    $resourceUrl = $this->setUpResourceFile($resourceFile);
+
+    // Build data-dictionary.
+    $dict_id = $uuid->generate();
+    $date_format = '%m/%d/%Y';
+    $fields = [
+      [
+        'name' => 'b',
+        'title' => 'b_title',
+        'type' => 'date',
+        'format' => $date_format,
+      ],
+    ];
+    $data_dict = $validMetadataFactory->get(
+      $this->getDataDictionary($fields, [], $dict_id),
+      'data-dictionary'
+    );
+    // Create data-dictionary.
+    $this->assertEquals(
+      $dict_id,
+      $metastore->post('data-dictionary', $data_dict)
+    );
+    // Publish should return FALSE, because the node was already published.
+    $this->assertFalse($metastore->publish('data-dictionary', $dict_id));
+    // Assert the date format is stored correctly.
+    $this->assertEquals(
+      $date_format,
+      $metastore->get('data-dictionary', $dict_id)->{'$.data.fields[0].format'}
+    );
+
+    // Build dataset.
+    $dataset_id = $uuid->generate();
+    $this->assertInstanceOf(
+      RootedJsonData::class,
+      $dataset = $validMetadataFactory->get(
+        $this->getDataset(
+          $dataset_id,
+          'Test ' . $dataset_id,
+          [$resourceUrl],
+          TRUE,
+          'dkan://metastore/schemas/data-dictionary/items/' . $dict_id
+        ),
+        'dataset'
+      )
+    );
+    // Create dataset.
+    $this->assertEquals(
+      $dataset_id,
+      $metastore->post('dataset', $dataset)
+    );
+    // Publish should return FALSE, because the node was already published.
+    $this->assertFalse($metastore->publish('dataset', $dataset_id));
+
+    // Retrieve dataset.
+    $this->assertInstanceOf(
+      RootedJsonData::class,
+      $dataset = $metastore->get('dataset', $dataset_id)
+    );
+    // The dataset references the dictionary. DescribedBy will contain the https
+    // URL-style reference.
+    $this->assertStringContainsString(
+      $dict_id,
+      $dataset->{'$["%Ref:distribution"][0].data.describedBy'}
+    );
+    // Get the distribution ID.
+    $distribution_id = $dataset->{'$["%Ref:distribution"][0].identifier'};
+
+    // Dictionary fields are applied to the dataset.
+    /** @var \Drupal\datastore\Service\ResourceProcessor\DictionaryEnforcer $dictionary_enforcer */
+    $dictionary_enforcer = $this->container->get('dkan.datastore.service.resource_processor.dictionary_enforcer');
+    $this->assertCount(
+      1,
+      $dictionary_fields = $dictionary_enforcer->returnDataDictionaryFields($distribution_id)
+    );
+    $this->assertEquals($date_format, $dictionary_fields[0]['format'] ?? 'not found');
+
+    // Set the dictionary CSV header mode before the import.
+    $this->config('metastore.settings')
+      ->set('csv_headers_mode', 'dictionary_titles')
+      ->save();
+
+    // Run queue items to perform the import.
+    $this->runQueues(['localize_import', 'datastore_import', 'post_import']);
+
+    // Query for the dataset, as a streaming CSV.
+    $client = $this->getHttpClient();
+    $response = $client->request(
+      'GET',
+      $this->baseUrl . '/api/1/datastore/query/' . $dataset_id . '/0/download',
+      ['query' => ['format' => 'csv']]
+    );
+
+    $lines = explode("\n", $response->getBody()->getContents());
+    // Header should be using the dictionary title.
+    $this->assertEquals('a,b_title,c,d,e', $lines[0]);
+
+    // Set the machine name CSV header mode.
+    $this->config('metastore.settings')
+      ->set('csv_headers_mode', 'machine_names')
+      ->save();
+
+    // Make another request.
+    $response = $client->request(
+      'GET',
+      $this->baseUrl . '/api/1/datastore/query/' . $dataset_id . '/0/download',
+      ['query' => ['format' => 'csv']]
+    );
+    $lines = explode("\n", $response->getBody()->getContents());
+    // Header should be using the machine name title.
+    $this->assertEquals('a,b,c,d,e', $lines[0]);
+    // Date value should use the dictionary format.
+    $this->assertEquals('1,02/23/1978,9.07,efghijk,0', $lines[1]);
+  }
+
+  /**
+   * Move a data test file to the public:// directory.
+   *
+   * @param string $resourceFile
+   *   The file name only of the data resource file.
+   *
+   * @return string
+   *   The resource URL of the moved resource.
+   *
+   * @todo Turn this into a trait or add it to a base class.
+   */
+  protected function setUpResourceFile(string $resourceFile) : string {
+    // Copy resource file to uploads directory.
+    /** @var \Drupal\Core\File\FileSystemInterface $file_system */
+    $file_system = $this->container->get('file_system');
+    $upload_path = $file_system->realpath(self::UPLOAD_LOCATION);
+    $file_system->prepareDirectory($upload_path, FileSystemInterface::CREATE_DIRECTORY);
+    $file_system->copy(self::TEST_DATA_PATH . $resourceFile, $upload_path, FileSystemInterface::EXISTS_REPLACE);
+    return $this->container->get('stream_wrapper_manager')
+      ->getViaUri(self::UPLOAD_LOCATION . $resourceFile)
+      ->getExternalUrl();
+  }
+
 }