From 6240985cec0d6a410b54bef0eced03c1f8d1fe60 Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Fri, 27 Dec 2024 17:24:37 -0500
Subject: [PATCH 01/23] Getting started on refactor.

---
 modules/common/src/DataResource.php           | 19 ++++++++++++-------
 .../src/Service/MysqlImport.php               |  8 ++++++--
 modules/datastore/src/DatastoreResource.php   |  9 +++++++--
 .../src/Plugin/QueueWorker/ImportJob.php      |  6 +++---
 .../datastore/src/Service/ImportService.php   | 17 ++++++++---------
 .../datastore/src/Storage/DatabaseTable.php   | 14 +++++++-------
 .../datastore/src/Storage/QueryFactory.php    |  2 +-
 7 files changed, 44 insertions(+), 31 deletions(-)

diff --git a/modules/common/src/DataResource.php b/modules/common/src/DataResource.php
index c5081a30dd..032fcb3cc8 100644
--- a/modules/common/src/DataResource.php
+++ b/modules/common/src/DataResource.php
@@ -135,9 +135,8 @@ public function __construct($file_path, $mimeType, $perspective = self::DEFAULT_
    * @return \Drupal\common\DataResource
    *   DataResource object.
    *
-   * @deprecated Use DataResource::createFromEntity() instead.
-   *
-   * @see self::createFromEntity()
+   * @deprecated in dkan:2.17.1 and is removed from dkan:2.20.0. Use DataResource::createFromEntity() instead.
+   * @see https://github.com/GetDKAN/dkan/pull/4027
    */
   public static function createFromRecord(object $record): DataResource {
     $resource = new static($record->filePath, $record->mimeType, $record->perspective);
@@ -253,10 +252,16 @@ public function getIdentifier() {
   }
 
   /**
-   * Getter.
+   * Get the resource's filepath.
+   *
+   * @param bool|null $resolve
+   *   Whether to resolve the URL host tokens in the file path.
+   *
+   * @return string
+   *   The file path.
    */
-  public function getFilePath() {
-    return $this->filePath;
+  public function getFilePath(?bool $resolve = FALSE):string {
+    return $resolve ? UrlHostTokenResolver::resolve($this->getFilePath()) : $this->filePath;
   }
 
   /**
@@ -333,7 +338,7 @@ public function getTableName() {
    * {@inheritDoc}
    */
   #[\ReturnTypeWillChange]
-  public function jsonSerialize() {
+  public function jsonSerialize(): mixed {
     return $this->serialize();
   }
 
diff --git a/modules/datastore/modules/datastore_mysql_import/src/Service/MysqlImport.php b/modules/datastore/modules/datastore_mysql_import/src/Service/MysqlImport.php
index c27a8e3227..e56cf7d9d2 100644
--- a/modules/datastore/modules/datastore_mysql_import/src/Service/MysqlImport.php
+++ b/modules/datastore/modules/datastore_mysql_import/src/Service/MysqlImport.php
@@ -68,8 +68,12 @@ protected function runIt() {
     }
 
     // Attempt to resolve resource file name from file path.
-    if (($file_path = \Drupal::service('file_system')->realpath($this->resource->getFilePath())) === FALSE) {
-      return $this->setResultError(sprintf('Unable to resolve file name "%s" for resource with identifier "%s".', $this->resource->getFilePath(), $this->resource->getId()));
+    if (($file_path = \Drupal::service('file_system')->realpath($this->resource->getFilePath(TRUE))) === FALSE) {
+      return $this->setResultError(sprintf(
+        'Unable to resolve file name "%s" for resource with identifier "%s".',
+        $this->resource->getFilePath(TRUE),
+        $this->resource->getUniqueIdentifier())
+      );
     }
 
     $size = @filesize($file_path);
diff --git a/modules/datastore/src/DatastoreResource.php b/modules/datastore/src/DatastoreResource.php
index 171d0b049e..13ca348f92 100644
--- a/modules/datastore/src/DatastoreResource.php
+++ b/modules/datastore/src/DatastoreResource.php
@@ -43,6 +43,8 @@ public function __construct($id, $file_path, $mime_type) {
 
   /**
    * Get the resource ID.
+   *
+   * Note: duplicates Drupal\common\DataResource::getUniqueIdentifier().
    */
   public function getId(): string {
     return $this->id;
@@ -50,6 +52,8 @@ public function getId(): string {
 
   /**
    * Get the file path.
+   *
+   * Note: duplicates Drupal\common\DataResource::getFilePath(TRUE).
    */
   public function getFilePath(): string {
     return $this->filePath;
@@ -57,6 +61,8 @@ public function getFilePath(): string {
 
   /**
    * Get the mimeType.
+   *
+   * Note: duplicates Drupal\common\DataResource::getMimeType().
    */
   public function getMimeType(): string {
     return $this->mimeType;
@@ -65,8 +71,7 @@ public function getMimeType(): string {
   /**
    * {@inheritdoc}
    */
-  #[\ReturnTypeWillChange]
-  public function jsonSerialize() {
+  public function jsonSerialize(): mixed {
     return (object) [
       'filePath' => $this->getFilePath(),
       'id' => $this->getId(),
diff --git a/modules/datastore/src/Plugin/QueueWorker/ImportJob.php b/modules/datastore/src/Plugin/QueueWorker/ImportJob.php
index e0b20deda0..2cbf05d561 100644
--- a/modules/datastore/src/Plugin/QueueWorker/ImportJob.php
+++ b/modules/datastore/src/Plugin/QueueWorker/ImportJob.php
@@ -89,7 +89,7 @@ class ImportJob extends AbstractPersistentJob {
   /**
    * Datastore resource.
    *
-   * @var \Drupal\datastore\DatastoreResource
+   * @var \Drupal\common\DataResource
    */
   protected $resource;
 
@@ -105,7 +105,7 @@ class ImportJob extends AbstractPersistentJob {
    * @param array|null $config
    *   Configuration options.
    */
-  protected function __construct(string $identifier, $storage, array $config = NULL) {
+  protected function __construct(string $identifier, $storage, ?array $config = NULL) {
     parent::__construct($identifier, $storage, $config);
 
     $this->dataStorage = $config['storage'];
@@ -195,7 +195,7 @@ public function getStorage() {
    * {@inheritdoc}
    */
   protected function runIt() {
-    $filename = $this->resource->getFilePath();
+    $filename = $this->resource->getFilePath(TRUE);
     $size = @filesize($filename);
     if (!$size) {
       return $this->setResultError("Can't get size from file {$filename}");
diff --git a/modules/datastore/src/Service/ImportService.php b/modules/datastore/src/Service/ImportService.php
index 87381d53a7..878e7dc1dd 100644
--- a/modules/datastore/src/Service/ImportService.php
+++ b/modules/datastore/src/Service/ImportService.php
@@ -151,10 +151,9 @@ public function import() {
     $data_resource = $this->getResource();
 
     if ($result->getStatus() === Result::ERROR) {
-      $datastore_resource = $data_resource->getDatastoreResource();
       $this->logger->error('Error importing resource id:%id path:%path message:%message', [
-        '%id' => $datastore_resource->getId(),
-        '%path' => $datastore_resource->getFilePath(),
+        '%id' => $data_resource->getUniqueIdentifier(),
+        '%path' => $data_resource->getFilePath(TRUE),
         '%message' => $result->getError(),
       ]);
     }
@@ -188,20 +187,20 @@ public function getImporter(): ImportJob {
     if ($this->importJob ?? FALSE) {
       return $this->importJob;
     }
-    $datastore_resource = $this->getResource()->getDatastoreResource();
+    $data_resource = $this->getResource();
 
     $delimiter = ",";
-    if ($datastore_resource->getMimeType() == 'text/tab-separated-values') {
+    if ($data_resource->getMimeType() == 'text/tab-separated-values') {
       $delimiter = "\t";
     }
 
     $this->importJob = call_user_func([$this->importerClass, 'get'],
-      $datastore_resource->getId(),
+      $data_resource->getUniqueIdentifier(),
       $this->importJobStoreFactory->getInstance(),
       [
         "storage" => $this->getStorage(),
         "parser" => $this->getNonRecordingParser($delimiter),
-        "resource" => $datastore_resource,
+        "resource" => $data_resource,
       ]
     );
 
@@ -245,8 +244,8 @@ private function getNonRecordingParser(string $delimiter) : Csv {
    *   DatabaseTable storage object.
    */
   public function getStorage(): DatabaseTable {
-    $datastore_resource = $this->getResource()->getDatastoreResource();
-    return $this->databaseTableFactory->getInstance($datastore_resource->getId(), ['resource' => $datastore_resource]);
+    $data_resource = $this->getResource();
+    return $this->databaseTableFactory->getInstance($data_resource->getUniqueIdentifier(), ['resource' => $data_resource]);
   }
 
   /**
diff --git a/modules/datastore/src/Storage/DatabaseTable.php b/modules/datastore/src/Storage/DatabaseTable.php
index ca7ab8fc8f..c56201fed6 100755
--- a/modules/datastore/src/Storage/DatabaseTable.php
+++ b/modules/datastore/src/Storage/DatabaseTable.php
@@ -3,8 +3,8 @@
 namespace Drupal\datastore\Storage;
 
 use Drupal\Core\Database\Connection;
+use Drupal\common\DataResource;
 use Drupal\common\Storage\AbstractDatabaseTable;
-use Drupal\datastore\DatastoreResource;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -17,7 +17,7 @@ class DatabaseTable extends AbstractDatabaseTable implements \JsonSerializable {
   /**
    * Datastore resource object.
    *
-   * @var \Drupal\datastore\DatastoreResource
+   * @var \Drupal\common\DataResource
    */
   private $resource;
 
@@ -31,15 +31,15 @@ class DatabaseTable extends AbstractDatabaseTable implements \JsonSerializable {
    *
    * @param \Drupal\Core\Database\Connection $connection
    *   Drupal database connection object.
-   * @param \Drupal\datastore\DatastoreResource $resource
+   * @param \Drupal\common\DataResource $resource
    *   A resource.
    * @param \Psr\Log\LoggerInterface $loggerChannel
    *   DKAN logger channel service.
    */
   public function __construct(
     Connection $connection,
-    DatastoreResource $resource,
-    LoggerInterface $loggerChannel
+    DataResource $resource,
+    LoggerInterface $loggerChannel,
   ) {
     // Set resource before calling the parent constructor. The parent calls
     // getTableName which we implement and needs the resource to operate.
@@ -76,7 +76,7 @@ public function getSummary() {
    * {@inheritdoc}
    */
   #[\ReturnTypeWillChange]
-  public function jsonSerialize() {
+  public function jsonSerialize(): mixed {
     return (object) ['resource' => $this->resource];
   }
 
@@ -88,7 +88,7 @@ public function jsonSerialize() {
    */
   public function getTableName() {
     if ($this->resource) {
-      return 'datastore_' . $this->resource->getId();
+      return 'datastore_' . $this->resource->getUniqueIdentifier();
     }
     return 'datastore_does_not_exist';
   }
diff --git a/modules/datastore/src/Storage/QueryFactory.php b/modules/datastore/src/Storage/QueryFactory.php
index 2ef54e5368..e113778072 100644
--- a/modules/datastore/src/Storage/QueryFactory.php
+++ b/modules/datastore/src/Storage/QueryFactory.php
@@ -40,7 +40,7 @@ public function __construct(DatastoreQuery $datastoreQuery, array $storageMap) {
   /**
    * Static factory create method.
    *
-   * @param Drupal\datastore\Service\DatastoreQuery $datastoreQuery
+   * @param \Drupal\datastore\Service\DatastoreQuery $datastoreQuery
    *   Datastore query request object.
    * @param array $storageMap
    *   Storage map array.

From ba482a63688261b6ccd5c0b5d21f0e95b135bb8f Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Mon, 30 Dec 2024 17:32:36 -0500
Subject: [PATCH 02/23] Fix getTableName

---
 modules/datastore/src/Storage/DatabaseTable.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/modules/datastore/src/Storage/DatabaseTable.php b/modules/datastore/src/Storage/DatabaseTable.php
index c56201fed6..8ec9daf890 100755
--- a/modules/datastore/src/Storage/DatabaseTable.php
+++ b/modules/datastore/src/Storage/DatabaseTable.php
@@ -88,7 +88,7 @@ public function jsonSerialize(): mixed {
    */
   public function getTableName() {
     if ($this->resource) {
-      return 'datastore_' . $this->resource->getUniqueIdentifier();
+      return $this->resource->getTableName();
     }
     return 'datastore_does_not_exist';
   }

From a0197734106cac28735e2cb0131d6700338ce5b3 Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Mon, 30 Dec 2024 18:15:18 -0500
Subject: [PATCH 03/23] getTableName should not live on DataResource

---
 modules/common/src/DataResource.php               |  4 ++++
 .../common/tests/src/Unit/DataResourceTest.php    | 15 ---------------
 modules/datastore/datastore.services.yml          |  1 +
 .../ResourceProcessor/DictionaryEnforcer.php      | 15 +++++++++++++--
 modules/datastore/src/Storage/DatabaseTable.php   |  2 +-
 .../Controller/QueryDownloadControllerTest.php    |  3 ++-
 6 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/modules/common/src/DataResource.php b/modules/common/src/DataResource.php
index 032fcb3cc8..c71cd866b0 100644
--- a/modules/common/src/DataResource.php
+++ b/modules/common/src/DataResource.php
@@ -235,6 +235,8 @@ public function changeMimeType($newMimeType) {
    *
    * @return \Drupal\datastore\DatastoreResource
    *   Datastore Resource.
+   *
+   * @deprecated
    */
   public function getDatastoreResource(): DatastoreResource {
     return new DatastoreResource(
@@ -329,6 +331,8 @@ public function getUniqueIdentifierNoPerspective(): string {
 
   /**
    * Retrieve datastore table name for resource.
+   *
+   * @deprecated
    */
   public function getTableName() {
     return 'datastore_' . md5($this->getUniqueIdentifier());
diff --git a/modules/common/tests/src/Unit/DataResourceTest.php b/modules/common/tests/src/Unit/DataResourceTest.php
index 078907b554..a2413f12fc 100644
--- a/modules/common/tests/src/Unit/DataResourceTest.php
+++ b/modules/common/tests/src/Unit/DataResourceTest.php
@@ -18,21 +18,6 @@
  */
 class DataResourceTest extends TestCase {
 
-  /**
-   * Test getTableName().
-   */
-  public function testGetTableName() {
-
-    $resource = new DataResource(
-      '/foo/bar',
-      'txt',
-      DataResource::DEFAULT_SOURCE_PERSPECTIVE
-    );
-    $tableName = $resource->getTableName();
-
-    $this->assertStringStartsWith('datastore_', $tableName);
-  }
-
   /**
    * Test getFolder().
    */
diff --git a/modules/datastore/datastore.services.yml b/modules/datastore/datastore.services.yml
index 3aef3143c4..e0889e2db2 100644
--- a/modules/datastore/datastore.services.yml
+++ b/modules/datastore/datastore.services.yml
@@ -42,6 +42,7 @@ services:
       - '@dkan.datastore.data_dictionary.alter_table_query_builder.mysql'
       - '@dkan.metastore.service'
       - '@dkan.metastore.data_dictionary_discovery'
+      - '@dkan.datastore.database_table_factory'
     tags:
       - { name: resource_processor, priority: 25 }
 
diff --git a/modules/datastore/src/Service/ResourceProcessor/DictionaryEnforcer.php b/modules/datastore/src/Service/ResourceProcessor/DictionaryEnforcer.php
index d3515db435..3bb3dfc8b1 100644
--- a/modules/datastore/src/Service/ResourceProcessor/DictionaryEnforcer.php
+++ b/modules/datastore/src/Service/ResourceProcessor/DictionaryEnforcer.php
@@ -5,6 +5,7 @@
 use Drupal\common\DataResource;
 use Drupal\datastore\DataDictionary\AlterTableQueryBuilderInterface;
 use Drupal\datastore\Service\ResourceProcessorInterface;
+use Drupal\datastore\Storage\DatabaseTableFactory;
 use Drupal\metastore\MetastoreService;
 use Drupal\metastore\DataDictionary\DataDictionaryDiscoveryInterface;
 
@@ -43,6 +44,13 @@ class DictionaryEnforcer implements ResourceProcessorInterface {
    */
   protected $resourceMapper;
 
+  /**
+   * Database table factory service.
+   *
+   * @var \Drupal\datastore\Storage\DatabaseTableFactory
+   */
+  protected $databaseTableFactory;
+
   /**
    * Constructs a \Drupal\Component\Plugin\PluginBase object.
    *
@@ -56,11 +64,13 @@ class DictionaryEnforcer implements ResourceProcessorInterface {
   public function __construct(
     AlterTableQueryBuilderInterface $alter_table_query_builder,
     MetastoreService $metastore,
-    DataDictionaryDiscoveryInterface $data_dictionary_discovery
+    DataDictionaryDiscoveryInterface $data_dictionary_discovery,
+    DatabaseTableFactory $table_factory,
   ) {
     $this->metastore = $metastore;
     $this->dataDictionaryDiscovery = $data_dictionary_discovery;
     $this->alterTableQueryBuilder = $alter_table_query_builder;
+    $this->databaseTableFactory = $table_factory;
   }
 
   /**
@@ -78,7 +88,8 @@ public function process(DataResource $resource): void {
     // Get data-dictionary for the given resource.
     $dictionary = $this->getDataDictionaryForResource($resource);
     // Retrieve name of datastore table for resource.
-    $datastore_table = $resource->getTableName();
+    $table = $this->databaseTableFactory->getInstance('', ['resource' => $resource]);
+    $datastore_table = $table->getTableName();
 
     $this->applyDictionary($dictionary, $datastore_table);
   }
diff --git a/modules/datastore/src/Storage/DatabaseTable.php b/modules/datastore/src/Storage/DatabaseTable.php
index 8ec9daf890..6af60b0b14 100755
--- a/modules/datastore/src/Storage/DatabaseTable.php
+++ b/modules/datastore/src/Storage/DatabaseTable.php
@@ -88,7 +88,7 @@ public function jsonSerialize(): mixed {
    */
   public function getTableName() {
     if ($this->resource) {
-      return $this->resource->getTableName();
+      return 'datastore_' . md5($this->resource->getUniqueIdentifier());
     }
     return 'datastore_does_not_exist';
   }
diff --git a/modules/datastore/tests/src/Unit/Controller/QueryDownloadControllerTest.php b/modules/datastore/tests/src/Unit/Controller/QueryDownloadControllerTest.php
index 94bf34fd01..be45419671 100644
--- a/modules/datastore/tests/src/Unit/Controller/QueryDownloadControllerTest.php
+++ b/modules/datastore/tests/src/Unit/Controller/QueryDownloadControllerTest.php
@@ -2,6 +2,7 @@
 
 namespace Drupal\Tests\datastore\Unit\Controller;
 
+use Drupal\common\DataResource;
 use Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher;
 use Drupal\Core\Cache\Context\CacheContextsManager;
 use Drupal\Core\Config\ConfigFactoryInterface;
@@ -475,7 +476,7 @@ public function mockDatastoreTable($connection, $id, $csvFile, $fields) {
 
     $storage = new SqliteDatabaseTable(
       $connection,
-      new DatastoreResource($id, "data-$id.csv", "text/csv"),
+      new DataResource("data-$id.csv", "text/csv"),
       $this->createStub(LoggerInterface::class)
     );
     $storage->setSchema([

From d4c20f3bef3fbd24b5dcd698fd469e102c82757a Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Mon, 6 Jan 2025 15:57:28 -0500
Subject: [PATCH 04/23] Make setTable() public

---
 modules/common/src/Storage/AbstractDatabaseTable.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/modules/common/src/Storage/AbstractDatabaseTable.php b/modules/common/src/Storage/AbstractDatabaseTable.php
index e477b179e0..f6170d57ae 100644
--- a/modules/common/src/Storage/AbstractDatabaseTable.php
+++ b/modules/common/src/Storage/AbstractDatabaseTable.php
@@ -263,7 +263,7 @@ protected function sanitizedErrorMessage(string $unsanitizedMessage) {
    * @throws \Exception
    *   Throws an exception if the schema was not already set.
    */
-  protected function setTable() {
+  public function setTable() {
     if (!$this->tableExist($table_name = $this->getTableName())) {
       if ($schema = $this->schema) {
         try {

From 084be88f90a51fc5b3436b5944811799cb313600 Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Mon, 6 Jan 2025 15:58:44 -0500
Subject: [PATCH 05/23] Fix QueryDownloadController unit test

---
 .../QueryDownloadControllerTest.php           | 79 +++++++++++--------
 1 file changed, 47 insertions(+), 32 deletions(-)

diff --git a/modules/datastore/tests/src/Unit/Controller/QueryDownloadControllerTest.php b/modules/datastore/tests/src/Unit/Controller/QueryDownloadControllerTest.php
index be45419671..79285bf7a9 100644
--- a/modules/datastore/tests/src/Unit/Controller/QueryDownloadControllerTest.php
+++ b/modules/datastore/tests/src/Unit/Controller/QueryDownloadControllerTest.php
@@ -34,8 +34,17 @@
  */
 class QueryDownloadControllerTest extends TestCase {
 
+  const FILE_DIR = __DIR__ . "/../../../data/";
+
   private $buffer;
 
+  /**
+   * Resources to be used in tests.
+   *
+   * @var \Drupal\common\DataResource[]
+   */
+  private array $resources;
+
   protected function setUp(): void {
     parent::setUp();
     // Set cache services
@@ -47,6 +56,11 @@ protected function setUp(): void {
       ->add(ContainerInterface::class, 'get', $options)
       ->add(CacheContextsManager::class, 'assertValidTokens', TRUE);
     \Drupal::setContainer($chain->getMock());
+
+    $this->resources = [
+      '2' => new DataResource(self::FILE_DIR . 'states_with_dupes.csv', 'text/csv'),
+      '3' => new DataResource(self::FILE_DIR . 'years_colors.csv', 'text/csv'),
+    ];
   }
 
   protected function tearDown(): void {
@@ -61,13 +75,13 @@ private function queryResultCompare($data, $resource = NULL) {
     $request = $this->mockRequest($data);
     $qController = QueryController::create($this->getQueryContainer(500));
     $response = $resource ? $qController->queryResource($resource, $request) : $qController->query($request);
-    $csv = $response->getContent();
+    $csv = $response->getContent() ?? '';
 
     $dController = QueryDownloadController::create($this->getQueryContainer(25));
     ob_start([self::class, 'getBuffer']);
     $streamResponse = $resource ? $dController->queryResource($resource, $request) : $dController->query($request);
     $streamResponse->sendContent();
-    $streamedCsv = $this->buffer;
+    $streamedCsv = $this->buffer ?? '';
     ob_get_clean();
 
     $this->assertEquals(count(explode("\n", $csv)), count(explode("\n", $streamedCsv)));
@@ -81,7 +95,7 @@ public function testStreamedQueryCsv() {
     $data = [
       "resources" => [
         [
-          "id" => "2",
+          "id" => $this->resources[2]->getIdentifier(),
           "alias" => "t",
         ],
       ],
@@ -109,7 +123,7 @@ public function testStreamedOtherSortCsv() {
     $data = [
       "resources" => [
         [
-          "id" => "2",
+          "id" => $this->resources[2]->getIdentifier(),
           "alias" => "t",
         ],
       ],
@@ -139,11 +153,11 @@ public function testStreamedJoinCsv() {
       "schema" => TRUE,
       "resources" => [
         [
-          "id" => "2",
+          "id" => $this->resources[2]->getIdentifier(),
           "alias" => "t",
         ],
         [
-          "id" => "3",
+          "id" => $this->resources[3]->getIdentifier(),
           "alias" => "j",
         ],
       ],
@@ -201,7 +215,7 @@ public function testStreamedQueryJson() {
     $data = json_encode([
       "resources" => [
         [
-          "id" => "2",
+          "id" => $this->resources[2]->getIdentifier(),
           "alias" => "t",
         ],
       ],
@@ -224,7 +238,7 @@ public function testStreamedLimit() {
     $data = json_encode([
       "resources" => [
         [
-          "id" => "2",
+          "id" => $this->resources[2]->getIdentifier(),
           "alias" => "t",
         ],
       ],
@@ -259,7 +273,7 @@ public function testStreamedCsvSpecificColumns() {
     $data = [
       "resources" => [
         [
-          "id" => "2",
+          "id" => $this->resources[2]->getIdentifier(),
           "alias" => "t",
         ],
       ],
@@ -276,7 +290,7 @@ public function testStreamedCsvResourceColumns() {
     $data = [
       "resources" => [
         [
-          "id" => "2",
+          "id" => $this->resources[2]->getIdentifier(),
           "alias" => "t",
         ],
       ],
@@ -304,7 +318,7 @@ public function testStreamedCsvRowIds() {
     $data = [
       "resources" => [
         [
-          "id" => "2",
+          "id" => $this->resources[2]->getIdentifier(),
           "alias" => "t",
         ],
       ],
@@ -322,7 +336,7 @@ public function testStreamedBadSchema() {
     $data = [
       "resources" => [
         [
-          "id" => "2",
+          "id" => $this->resources[2]->getIdentifier(),
           "alias" => "tx",
         ],
       ],
@@ -394,13 +408,13 @@ private function getQueryContainer(int $rowLimit, ?int $responseStreamMaxAge = N
       ],
     ];
 
-    $storage2 = $this->mockDatastoreTable($connection, "2", 'states_with_dupes.csv', $schema2);
+    $storage2 = $this->mockDatastoreTable($connection, $this->resources[2], $schema2);
     $storage2x = clone($storage2);
     $storage2x->setSchema(['fields' => []]);
     $storageMap = [
       't' => $storage2,
       'tx' => $storage2x,
-      'j' => $this->mockDatastoreTable($connection, "3", 'years_colors.csv', $schema3
+      'j' => $this->mockDatastoreTable($connection, $this->resources[3], $schema3
       ),
     ];
 
@@ -415,7 +429,7 @@ private function getQueryContainer(int $rowLimit, ?int $responseStreamMaxAge = N
       ->add(Data::class, 'getCacheMaxAge', 0)
       ->add(ConfigFactoryInterface::class, 'get', ImmutableConfig::class)
       ->add(Query::class, "getQueryStorageMap", $storageMap)
-      ->add(Query::class, 'getDatastoreService',  DatastoreService::class)
+      ->add(Query::class, 'getDatastoreService', DatastoreService::class)
       ->add(DatastoreService::class, 'getDataDictionaryFields', NULL)
       // @todo Use an Options or Sequence return here; this will only work for one arg at a time.
       ->add(ImmutableConfig::class, 'get', $rowLimit)
@@ -451,37 +465,38 @@ public function mockRequest($data = '') {
    * @return \Drupal\common\Storage\DatabaseTableInterface
    *   A database table storage class useable for datastore queries.
    */
-  public function mockDatastoreTable($connection, $id, $csvFile, $fields) {
-    foreach ($fields as $name => $field) {
+  public function mockDatastoreTable($connection, DataResource $resource, $fields) {
+    $storage = new SqliteDatabaseTable(
+      $connection,
+      $resource,
+      $this->createStub(LoggerInterface::class)
+    );
+    $storage->setSchema([
+      'fields' => $fields,
+    ]);
+    $storage->setTable();
+
+    foreach ($fields as $field) {
       $types[] = $field['type'];
-      $notNull = $field['not null'] ?? FALSE;
-      $createFields[] = "`$name` " . strtoupper($field['type']) . (($notNull) ? ' NOT NULL' : '');
     }
-    $createFieldsStr = implode(", ", $createFields);
-    $connection->query("CREATE TABLE `datastore_$id` ($createFieldsStr);");
-
+    
+    $fp = fopen($resource->getFilePath(), 'rb');
     $sampleData = [];
-    $fp = fopen(__DIR__ . "/../../../data/$csvFile", 'rb');
     while (!feof($fp)) {
       $sampleData[] = fgetcsv($fp);
     }
+    fclose($fp);
+
+    $table_name = $storage->getTableName();
     foreach ($sampleData as $row) {
       $values = [];
       foreach ($row as $key => $value) {
         $values[] = $types[$key] == "int" ? $value : "'$value'";
         $valuesStr = implode(", ", $values);
       }
-      $connection->query("INSERT INTO `datastore_$id` VALUES ($valuesStr);");
+      $connection->query("INSERT INTO `$table_name` VALUES ($valuesStr);");
     }
 
-    $storage = new SqliteDatabaseTable(
-      $connection,
-      new DataResource("data-$id.csv", "text/csv"),
-      $this->createStub(LoggerInterface::class)
-    );
-    $storage->setSchema([
-      'fields' => $fields,
-    ]);
     return $storage;
   }
 

From 50df2c1793b9e0abc3fadee778dfebfdf351118f Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Tue, 7 Jan 2025 10:29:28 -0500
Subject: [PATCH 06/23] Fix QueryControllerTest

---
 .../Unit/Controller/QueryControllerTest.php   | 66 ++++++++++++-------
 .../QueryDownloadControllerTest.php           | 22 ++++---
 2 files changed, 55 insertions(+), 33 deletions(-)

diff --git a/modules/datastore/tests/src/Unit/Controller/QueryControllerTest.php b/modules/datastore/tests/src/Unit/Controller/QueryControllerTest.php
index 541ed5b1cd..3427fc231b 100644
--- a/modules/datastore/tests/src/Unit/Controller/QueryControllerTest.php
+++ b/modules/datastore/tests/src/Unit/Controller/QueryControllerTest.php
@@ -2,10 +2,12 @@
 
 namespace Drupal\Tests\datastore\Unit\Controller;
 
+use Drupal\common\DataResource;
 use Drupal\Core\Cache\Context\CacheContextsManager;
 use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\Config\ImmutableConfig;
 use Drupal\common\DatasetInfo;
+use Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher;
 use Drupal\datastore\Controller\QueryController;
 use Drupal\datastore\DatastoreResource;
 use Drupal\datastore\DatastoreService;
@@ -33,23 +35,33 @@
  */
 class QueryControllerTest extends TestCase {
 
+  const FILE_DIR = __DIR__ . "/../../../data/";
+
+  /**
+   * Resources to be used in tests.
+   */
+  private DataResource $resource;
+
   protected function setUp(): void {
     parent::setUp();
     // Set cache services.
     $options = (new Options)
       ->add('cache_contexts_manager', CacheContextsManager::class)
+      ->add('event_dispatcher', ContainerAwareEventDispatcher::class)
       ->index(0);
     $chain = (new Chain($this))
       ->add(ContainerInterface::class, 'get', $options)
       ->add(CacheContextsManager::class, 'assertValidTokens', TRUE);
     \Drupal::setContainer($chain->getMock());
+
+    $this->resource = new DataResource(self::FILE_DIR . 'states_with_dupes.csv', 'text/csv');
   }
 
   public function testQueryJson() {
     $data = json_encode([
       "resources" => [
         [
-          "id" => "2",
+          "id" => $this->resource->getIdentifier(),
           "alias" => "t",
         ],
       ],
@@ -98,7 +110,7 @@ public function testQueryRowIdProperty() {
     // Try simple string properties:
     $data = json_encode(["properties" => ["record_number", "state"]]);
 
-    $result = $this->getQueryResult($data, "2");
+    $result = $this->getQueryResult($data, $this->resource->getIdentifier());
 
     $this->assertEquals(400, $result->getStatusCode());
     $this->assertStringContainsString('The record_number property is for internal use', $result->getContent());
@@ -106,7 +118,7 @@ public function testQueryRowIdProperty() {
     // Now try with rowIds plus an arbitrary property:
     $data = json_encode(["properties" => ["state"], "rowIds" => TRUE]);
 
-    $result = $this->getQueryResult($data, "2");
+    $result = $this->getQueryResult($data, $this->resource->getIdentifier());
 
     $this->assertEquals(400, $result->getStatusCode());
     $this->assertStringContainsString('The rowIds property cannot be set to true', $result->getContent());
@@ -125,7 +137,7 @@ public function testQueryRowIdProperty() {
       ],
     ]);
 
-    $result = $this->getQueryResult($data, "2");
+    $result = $this->getQueryResult($data, $this->resource->getIdentifier());
 
     $this->assertEquals(400, $result->getStatusCode());
     $this->assertStringContainsString('The record_number property is for internal use', $result->getContent());
@@ -147,7 +159,7 @@ public function testQueryRowIdSort() {
       ],
     ]);
 
-    $result = $this->getQueryResult($data, "2");
+    $result = $this->getQueryResult($data, $this->resource->getIdentifier());
 
     $this->assertTrue($result instanceof JsonResponse);
     $this->assertEquals(200, $result->getStatusCode());
@@ -195,7 +207,7 @@ public function testResourceQueryInvalidQuery() {
     $data = json_encode([
       "conditions" => "nope",
     ]);
-    $result = $this->getQueryResult($data, "2");
+    $result = $this->getQueryResult($data, $this->resource->getIdentifier());
 
     $this->assertTrue($result instanceof JsonResponse);
     $this->assertEquals(400, $result->getStatusCode());
@@ -208,7 +220,7 @@ public function testResourceQueryWithJoin() {
         "condition" => "t.field1 = s.field1",
       ],
     ]);
-    $result = $this->getQueryResult($data, "2");
+    $result = $this->getQueryResult($data, $this->resource->getIdentifier());
 
     $this->assertTrue($result instanceof JsonResponse);
     $this->assertEquals(400, $result->getStatusCode());
@@ -221,7 +233,7 @@ public function testResourceQueryJson() {
     $data = json_encode([
       "results" => TRUE,
     ]);
-    $result = $this->getQueryResult($data, "2");
+    $result = $this->getQueryResult($data, $this->resource->getIdentifier());
 
     $this->assertTrue($result instanceof JsonResponse);
     $this->assertEquals(200, $result->getStatusCode());
@@ -274,7 +286,7 @@ public function testResourceQueryJoins() {
         ],
       ],
     ]);
-    $result = $this->getQueryResult($data, "2");
+    $result = $this->getQueryResult($data, $this->resource->getIdentifier());
 
     $this->assertTrue($result instanceof JsonResponse);
     $this->assertEquals(400, $result->getStatusCode());
@@ -288,7 +300,7 @@ public function testQueryCsv() {
     $data = json_encode([
       "resources" => [
         [
-          "id" => "2",
+          "id" => $this->resource->getIdentifier(),
           "alias" => "t",
         ],
       ],
@@ -330,7 +342,7 @@ public function testResourceQueryCsv() {
       "results" => TRUE,
       "format" => "csv",
     ]);
-    $result = $this->getQueryResult($data, "2");
+    $result = $this->getQueryResult($data, $this->resource->getIdentifier());
     $this->assertTrue($result instanceof CsvResponse);
     $csv = explode("\n", $result->getContent());
     $this->assertEquals('State', $csv[0]);
@@ -358,7 +370,7 @@ public function testResourceExpressionQueryCsv() {
       "results" => TRUE,
       "format" => "csv",
     ]);
-    $result = $this->getQueryResult($data, "2");
+    $result = $this->getQueryResult($data, $this->resource->getIdentifier());
     $this->assertTrue($result instanceof CsvResponse);
 
     $csv = explode("\n", $result->getContent());
@@ -428,7 +440,7 @@ public function testQueryCsvCacheHeaders() {
     $data = json_encode([
       "resources" => [
         [
-          "id" => "2",
+          "id" => $this->resource->getIdentifier(),
           "alias" => "t",
         ],
       ],
@@ -524,20 +536,10 @@ public function mockRequest($data = '') {
    */
   public function mockDatastoreTable() {
     $connection = new SqliteConnection(new \PDO('sqlite::memory:'), []);
-    $connection->query('CREATE TABLE `datastore_2` (`record_number` INTEGER NOT NULL, state TEXT, year INT);');
-
-    $sampleData = [];
-    $fp = fopen(__DIR__ . '/../../../data/states_with_dupes.csv', 'rb');
-    while (!feof($fp)) {
-      $sampleData[] = fgetcsv($fp);
-    }
-    foreach ($sampleData as $row) {
-      $connection->query("INSERT INTO `datastore_2` VALUES ($row[0], '$row[1]', $row[2]);");
-    }
-
+    // $connection->query('CREATE TABLE `datastore_2` (`record_number` INTEGER NOT NULL, state TEXT, year INT);');
     $storage = new SqliteDatabaseTable(
       $connection,
-      new DatastoreResource("2", "data.csv", "text/csv"),
+      $this->resource,
       $this->createStub(LoggerInterface::class)
     );
     $storage->setSchema([
@@ -547,6 +549,20 @@ public function mockDatastoreTable() {
         'year' => ['type' => 'int', 'description' => 'Year'],
       ],
     ]);
+    $storage->setTable();
+
+    $fp = fopen($this->resource->getFilePath(), 'rb');
+    $sampleData = [];
+    while (!feof($fp)) {
+      $sampleData[] = fgetcsv($fp);
+    }
+    fclose($fp);
+
+    $table_name = $storage->getTableName();
+    foreach ($sampleData as $row) {
+      $connection->query("INSERT INTO `$table_name` VALUES ($row[0], '$row[1]', $row[2]);");
+    }
+
     return $storage;
   }
 
diff --git a/modules/datastore/tests/src/Unit/Controller/QueryDownloadControllerTest.php b/modules/datastore/tests/src/Unit/Controller/QueryDownloadControllerTest.php
index 79285bf7a9..7c03e02bab 100644
--- a/modules/datastore/tests/src/Unit/Controller/QueryDownloadControllerTest.php
+++ b/modules/datastore/tests/src/Unit/Controller/QueryDownloadControllerTest.php
@@ -36,7 +36,12 @@ class QueryDownloadControllerTest extends TestCase {
 
   const FILE_DIR = __DIR__ . "/../../../data/";
 
-  private $buffer;
+  /**
+   * Output buffer.
+   *
+   * @var string
+   */
+  private string $buffer;
 
   /**
    * Resources to be used in tests.
@@ -375,8 +380,6 @@ private function getQueryContainer(int $rowLimit, ?int $responseStreamMaxAge = N
       ->add('dkan.metastore.api_response', MetastoreApiResponse::class)
       ->index(0);
 
-    $connection = new SqliteConnection(new \PDO('sqlite::memory:'), []);
-
     $schema2 = [
       'record_number' => [
         'type' => 'int',
@@ -408,13 +411,13 @@ private function getQueryContainer(int $rowLimit, ?int $responseStreamMaxAge = N
       ],
     ];
 
-    $storage2 = $this->mockDatastoreTable($connection, $this->resources[2], $schema2);
+    $storage2 = $this->mockDatastoreTable($this->resources[2], $schema2);
     $storage2x = clone($storage2);
     $storage2x->setSchema(['fields' => []]);
     $storageMap = [
       't' => $storage2,
       'tx' => $storage2x,
-      'j' => $this->mockDatastoreTable($connection, $this->resources[3], $schema3
+      'j' => $this->mockDatastoreTable($this->resources[3], $schema3
       ),
     ];
 
@@ -465,7 +468,9 @@ public function mockRequest($data = '') {
    * @return \Drupal\common\Storage\DatabaseTableInterface
    *   A database table storage class useable for datastore queries.
    */
-  public function mockDatastoreTable($connection, DataResource $resource, $fields) {
+  public function mockDatastoreTable(DataResource $resource, $fields) {
+    $connection = new SqliteConnection(new \PDO('sqlite::memory:'), []);
+
     $storage = new SqliteDatabaseTable(
       $connection,
       $resource,
@@ -503,9 +508,10 @@ public function mockDatastoreTable($connection, DataResource $resource, $fields)
   /**
    * Callback to get output buffer.
    *
-   * @param $buffer
+   * @param string $buffer
+   *   A buffer to be appended to existing buffer in memory.
    */
-  protected function getBuffer($buffer) {
+  protected function getBuffer(string $buffer) {
     $this->buffer .= $buffer;
   }
 

From 482af9b77aab83678aa76ae3d949699b24f83dc0 Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Tue, 7 Jan 2025 13:03:35 -0500
Subject: [PATCH 07/23] More test fixes

---
 .../src/Storage/AbstractDatabaseTable.php     |  9 ++++++-
 .../datastore/src/Storage/DatabaseTable.php   |  5 ++--
 .../tests/data/states_with_dupes_link.csv     |  1 +
 .../QueryDownloadControllerTest.php           | 24 ++++++++++---------
 4 files changed, 25 insertions(+), 14 deletions(-)
 create mode 120000 modules/datastore/tests/data/states_with_dupes_link.csv

diff --git a/modules/common/src/Storage/AbstractDatabaseTable.php b/modules/common/src/Storage/AbstractDatabaseTable.php
index f6170d57ae..1c95cabadf 100644
--- a/modules/common/src/Storage/AbstractDatabaseTable.php
+++ b/modules/common/src/Storage/AbstractDatabaseTable.php
@@ -20,6 +20,11 @@ abstract class AbstractDatabaseTable implements DatabaseTableInterface {
    */
   const EVENT_TABLE_CREATE = 'dkan_common_table_create';
 
+  /**
+   * Real table name.
+   */
+  protected string $tableName;
+
   /**
    * A schema. Should be a drupal schema array.
    *
@@ -40,7 +45,7 @@ abstract class AbstractDatabaseTable implements DatabaseTableInterface {
    * @return string
    *   Table name.
    */
-  abstract protected function getTableName();
+  abstract public function getTableName();
 
   /**
    * Prepare data.
@@ -68,6 +73,7 @@ public function __construct(Connection $connection) {
 
     if ($this->tableExist($this->getTableName())) {
       $this->setSchemaFromTable();
+      $this->tableName = $this->getTableName();
     }
   }
 
@@ -268,6 +274,7 @@ public function setTable() {
       if ($schema = $this->schema) {
         try {
           $this->tableCreate($table_name, $schema);
+          $this->tableName = $table_name;
         }
         catch (SchemaObjectExistsException) {
           // Table already exists, which is totally OK. Other throwables find
diff --git a/modules/datastore/src/Storage/DatabaseTable.php b/modules/datastore/src/Storage/DatabaseTable.php
index 6af60b0b14..3536a32eb1 100755
--- a/modules/datastore/src/Storage/DatabaseTable.php
+++ b/modules/datastore/src/Storage/DatabaseTable.php
@@ -19,12 +19,12 @@ class DatabaseTable extends AbstractDatabaseTable implements \JsonSerializable {
    *
    * @var \Drupal\common\DataResource
    */
-  private $resource;
+  protected $resource;
 
   /**
    * DKAN logger channel service.
    */
-  private LoggerInterface $logger;
+  protected LoggerInterface $logger;
 
   /**
    * Constructor method.
@@ -49,6 +49,7 @@ public function __construct(
 
     if ($this->tableExist($this->getTableName())) {
       $this->setSchemaFromTable();
+      $this->tableName = $this->getTableName();
     }
   }
 
diff --git a/modules/datastore/tests/data/states_with_dupes_link.csv b/modules/datastore/tests/data/states_with_dupes_link.csv
new file mode 120000
index 0000000000..3a2a7767d4
--- /dev/null
+++ b/modules/datastore/tests/data/states_with_dupes_link.csv
@@ -0,0 +1 @@
+states_with_dupes.csv
\ No newline at end of file
diff --git a/modules/datastore/tests/src/Unit/Controller/QueryDownloadControllerTest.php b/modules/datastore/tests/src/Unit/Controller/QueryDownloadControllerTest.php
index 7c03e02bab..fae930e3d5 100644
--- a/modules/datastore/tests/src/Unit/Controller/QueryDownloadControllerTest.php
+++ b/modules/datastore/tests/src/Unit/Controller/QueryDownloadControllerTest.php
@@ -10,7 +10,6 @@
 use Drupal\common\DatasetInfo;
 use Drupal\datastore\Controller\QueryController;
 use Drupal\datastore\Controller\QueryDownloadController;
-use Drupal\datastore\DatastoreResource;
 use Drupal\datastore\DatastoreService;
 use Drupal\datastore\Service\Query;
 use Drupal\datastore\Storage\SqliteDatabaseTable;
@@ -65,12 +64,15 @@ protected function setUp(): void {
     $this->resources = [
       '2' => new DataResource(self::FILE_DIR . 'states_with_dupes.csv', 'text/csv'),
       '3' => new DataResource(self::FILE_DIR . 'years_colors.csv', 'text/csv'),
+      '4' => new DataResource(self::FILE_DIR . 'states_with_dupes_link.csv', 'text/csv'),
     ];
+
+    $this->buffer = '';
   }
 
   protected function tearDown(): void {
     parent::tearDown();
-    $this->buffer = NULL;
+    $this->buffer = '';
   }
 
   /**
@@ -362,14 +364,15 @@ public function testStreamedBadSchema() {
    * Create a mock object for the main container passed to the controller.
    *
    * @param int $rowLimit
-   *    The row limit for a query.
+   *   The row limit for a query.
    * @param int|null $responseStreamMaxAge
-   *    The max age for the response stream in cache, or NULL to use the default.
+   *   The max age for the response stream in cache, or NULL to use the default.
    *
    * @return \PHPUnit\Framework\MockObject\MockObject
    *   MockChain mock object.
    */
   private function getQueryContainer(int $rowLimit, ?int $responseStreamMaxAge = NULL) {
+    $connection = new SqliteConnection(new \PDO('sqlite::memory:'), []);
     $options = (new Options())
       ->add("dkan.metastore.storage", DataFactory::class)
       ->add("dkan.datastore.service", DatastoreService::class)
@@ -411,14 +414,14 @@ private function getQueryContainer(int $rowLimit, ?int $responseStreamMaxAge = N
       ],
     ];
 
-    $storage2 = $this->mockDatastoreTable($this->resources[2], $schema2);
-    $storage2x = clone($storage2);
+    $storage2 = $this->mockDatastoreTable($this->resources[2], $schema2, $connection);
+    $storage2x = $this->mockDatastoreTable($this->resources[4], $schema2, $connection);
     $storage2x->setSchema(['fields' => []]);
+    $storage3 = $this->mockDatastoreTable($this->resources[3], $schema3, $connection);
     $storageMap = [
       't' => $storage2,
       'tx' => $storage2x,
-      'j' => $this->mockDatastoreTable($this->resources[3], $schema3
-      ),
+      'j' => $storage3,
     ];
 
     $chain = (new Chain($this))
@@ -468,9 +471,8 @@ public function mockRequest($data = '') {
    * @return \Drupal\common\Storage\DatabaseTableInterface
    *   A database table storage class useable for datastore queries.
    */
-  public function mockDatastoreTable(DataResource $resource, $fields) {
-    $connection = new SqliteConnection(new \PDO('sqlite::memory:'), []);
-
+  public function mockDatastoreTable(DataResource $resource, $fields, $connection) {
+    
     $storage = new SqliteDatabaseTable(
       $connection,
       $resource,

From 3ab30ca652e7910ed09b77b28cfd983a68bcd029 Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Tue, 7 Jan 2025 13:13:18 -0500
Subject: [PATCH 08/23] Revert public getTableName function

---
 modules/common/src/Storage/AbstractDatabaseTable.php | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/modules/common/src/Storage/AbstractDatabaseTable.php b/modules/common/src/Storage/AbstractDatabaseTable.php
index 1c95cabadf..e9268fc59c 100644
--- a/modules/common/src/Storage/AbstractDatabaseTable.php
+++ b/modules/common/src/Storage/AbstractDatabaseTable.php
@@ -39,14 +39,6 @@ abstract class AbstractDatabaseTable implements DatabaseTableInterface {
    */
   protected $connection;
 
-  /**
-   * Get the full name of datastore db table.
-   *
-   * @return string
-   *   Table name.
-   */
-  abstract public function getTableName();
-
   /**
    * Prepare data.
    *
@@ -73,7 +65,6 @@ public function __construct(Connection $connection) {
 
     if ($this->tableExist($this->getTableName())) {
       $this->setSchemaFromTable();
-      $this->tableName = $this->getTableName();
     }
   }
 
@@ -274,7 +265,6 @@ public function setTable() {
       if ($schema = $this->schema) {
         try {
           $this->tableCreate($table_name, $schema);
-          $this->tableName = $table_name;
         }
         catch (SchemaObjectExistsException) {
           // Table already exists, which is totally OK. Other throwables find

From aaf9ece84163247fd7e5fd8002ac8a99d81aaa81 Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Tue, 7 Jan 2025 13:46:32 -0500
Subject: [PATCH 09/23] Fix more tests

---
 .../DictionaryEnforcerTest.php                |  1 +
 .../src/Kernel/Storage/DatabaseTableTest.php  |  4 +-
 .../DictionaryEnforcerTest.php                | 38 +++++++++++++++++--
 .../src/Unit/Storage/DatabaseTableTest.php    |  3 +-
 4 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/modules/datastore/tests/src/Kernel/Service/ResourceProcessor/DictionaryEnforcerTest.php b/modules/datastore/tests/src/Kernel/Service/ResourceProcessor/DictionaryEnforcerTest.php
index bfebeb3878..cf5918dce5 100644
--- a/modules/datastore/tests/src/Kernel/Service/ResourceProcessor/DictionaryEnforcerTest.php
+++ b/modules/datastore/tests/src/Kernel/Service/ResourceProcessor/DictionaryEnforcerTest.php
@@ -66,6 +66,7 @@ public function testProcessModeNone() {
         $this->container->get('dkan.datastore.data_dictionary.alter_table_query_builder.mysql'),
         $this->container->get('dkan.metastore.service'),
         $this->container->get('dkan.metastore.data_dictionary_discovery'),
+        $this->container->get('dkan.datastore.database_table_factory'),
       ])
       ->onlyMethods(['getDataDictionaryForResource', 'applyDictionary'])
       ->getMock();
diff --git a/modules/datastore/tests/src/Kernel/Storage/DatabaseTableTest.php b/modules/datastore/tests/src/Kernel/Storage/DatabaseTableTest.php
index 2816b4e793..1c9dda48bf 100644
--- a/modules/datastore/tests/src/Kernel/Storage/DatabaseTableTest.php
+++ b/modules/datastore/tests/src/Kernel/Storage/DatabaseTableTest.php
@@ -9,6 +9,7 @@
 use Drupal\datastore\Plugin\QueueWorker\ImportJob;
 use Drupal\datastore\Service\Factory\ImportServiceFactory;
 use Drupal\datastore\Storage\DatabaseTable;
+use Drupal\datastore\Storage\DatabaseTableFactory;
 use Drupal\KernelTests\KernelTestBase;
 use Drupal\Tests\common\Unit\Connection;
 use Procrastinator\Result;
@@ -95,8 +96,9 @@ public function testPrepareDataLogging($expected_log, $expected_exception, $data
       ->onlyMethods(['tableExist'])
       ->setConstructorArgs([
         $this->createStub(Connection::class),
-        $this->createStub(DatastoreResource::class),
+        $this->createStub(DataResource::class),
         $logger,
+        $this->createStub(DatabaseTableFactory::class),
       ])
       ->getMock();
     $database_table->expects($this->any())
diff --git a/modules/datastore/tests/src/Unit/Service/ResourceProcessor/DictionaryEnforcerTest.php b/modules/datastore/tests/src/Unit/Service/ResourceProcessor/DictionaryEnforcerTest.php
index 84c9d2d418..ee0631a871 100644
--- a/modules/datastore/tests/src/Unit/Service/ResourceProcessor/DictionaryEnforcerTest.php
+++ b/modules/datastore/tests/src/Unit/Service/ResourceProcessor/DictionaryEnforcerTest.php
@@ -14,6 +14,9 @@
 use Drupal\datastore\Service\PostImport;
 use Drupal\datastore\Service\ResourceProcessorCollector;
 use Drupal\datastore\Service\ResourceProcessor\DictionaryEnforcer;
+use Drupal\datastore\Storage\DatabaseTable;
+use Drupal\datastore\Storage\DatabaseTableFactory;
+use Drupal\jsonapi\JsonApiResource\Data;
 use Drupal\metastore\DataDictionary\DataDictionaryDiscovery;
 use Drupal\metastore\DataDictionary\DataDictionaryDiscoveryInterface;
 use Drupal\metastore\MetastoreService;
@@ -59,7 +62,17 @@ public function testProcess() {
     $dictionary_discovery_service = (new Chain($this))
       ->add(DataDictionaryDiscoveryInterface::class, 'dictionaryIdFromResource', 'dictionary-id')
       ->getMock();
-    $dictionary_enforcer = new DictionaryEnforcer($alter_table_query_builder, $metastore_service, $dictionary_discovery_service);
+    $database_table_factory = (new Chain($this))
+      ->add(DatabaseTableFactory::class, 'getInstance', DatabaseTable::class)
+      ->add(DatabaseTable::class, 'getTableName', 'datastore_table')
+      ->getMock();
+
+    $dictionary_enforcer = new DictionaryEnforcer(
+      $alter_table_query_builder,
+      $metastore_service,
+      $dictionary_discovery_service,
+      $database_table_factory
+    );
 
     $container_chain = $this->getContainerChain($resource->getVersion())
       ->add(AlterTableQueryInterface::class, 'execute')
@@ -96,7 +109,16 @@ public function testProcessItemExecuteException() {
     $dictionary_discovery_service = (new Chain($this))
       ->add(DataDictionaryDiscoveryInterface::class, 'dictionaryIdFromResource', 'data-dictionary')
       ->getMock();
-    $dictionary_enforcer = new DictionaryEnforcer($alter_table_query_builder, $metastore_service, $dictionary_discovery_service);
+    $database_table_factory = (new Chain($this))
+      ->add(DatabaseTableFactory::class, 'getInstance', DatabaseTable::class)
+      ->add(DatabaseTable::class, 'getTableName', 'datastore_table')
+      ->getMock();
+    $dictionary_enforcer = new DictionaryEnforcer(
+      $alter_table_query_builder,
+      $metastore_service,
+      $dictionary_discovery_service,
+      $database_table_factory
+    );
 
     $container_chain = $this->getContainerChain($resource->getVersion())
       ->add(AlterTableQueryInterface::class, 'execute')
@@ -133,7 +155,17 @@ public function testReturnDataDictionaryFields() {
       ->add(DataDictionaryDiscoveryInterface::class, 'getDataDictionaryMode', DataDictionaryDiscoveryInterface::MODE_SITEWIDE)
       ->add(DataDictionaryDiscoveryInterface::class, 'getSitewideDictionaryId','2')
       ->getMock();
-    $dictionary_enforcer = new DictionaryEnforcer($alter_table_query_builder, $metastore_service, $dictionary_discovery_service);
+    $database_table_factory = (new Chain($this))
+      ->add(DatabaseTableFactory::class, 'getInstance', DatabaseTable::class)
+      ->add(DatabaseTable::class, 'getTableName', 'datastore_table')
+      ->getMock();
+
+    $dictionary_enforcer = new DictionaryEnforcer(
+      $alter_table_query_builder,
+      $metastore_service,
+      $dictionary_discovery_service,
+      $database_table_factory
+    );
 
     $container_chain = $this->getContainerChain($resource->getVersion())
       ->add(AlterTableQueryInterface::class, 'execute')
diff --git a/modules/datastore/tests/src/Unit/Storage/DatabaseTableTest.php b/modules/datastore/tests/src/Unit/Storage/DatabaseTableTest.php
index e936432e80..4a87ce7148 100644
--- a/modules/datastore/tests/src/Unit/Storage/DatabaseTableTest.php
+++ b/modules/datastore/tests/src/Unit/Storage/DatabaseTableTest.php
@@ -2,6 +2,7 @@
 
 namespace Drupal\Tests\datastore\Storage;
 
+use Drupal\common\DataResource;
 use Drupal\Core\Database\Connection;
 use Drupal\Core\Database\DatabaseExceptionWrapper;
 use Drupal\Core\Database\Query\Insert;
@@ -476,7 +477,7 @@ private function getConnectionChain() {
    * Private.
    */
   private function getResource() {
-    return new DatastoreResource("people", "", "text/csv");
+    return new DataResource("", "text/csv");
   }
 
 }

From c3c8b3ec4ac00a48f4546c93a4661849a3101699 Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Tue, 7 Jan 2025 14:31:14 -0500
Subject: [PATCH 10/23] Fix MySqlDatabaseTableFactoryTest

---
 .../src/Kernel/Storage/MySqlDatabaseTableFactoryTest.php   | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/modules/datastore/modules/datastore_mysql_import/tests/src/Kernel/Storage/MySqlDatabaseTableFactoryTest.php b/modules/datastore/modules/datastore_mysql_import/tests/src/Kernel/Storage/MySqlDatabaseTableFactoryTest.php
index 92f8470c40..a86ae5baaf 100644
--- a/modules/datastore/modules/datastore_mysql_import/tests/src/Kernel/Storage/MySqlDatabaseTableFactoryTest.php
+++ b/modules/datastore/modules/datastore_mysql_import/tests/src/Kernel/Storage/MySqlDatabaseTableFactoryTest.php
@@ -2,7 +2,7 @@
 
 namespace Drupal\Tests\datastore_mysql_import\Kernel\Storage;
 
-use Drupal\datastore\DatastoreResource;
+use Drupal\common\DataResource;
 use Drupal\datastore_mysql_import\Storage\MySqlDatabaseTable;
 use Drupal\KernelTests\KernelTestBase;
 
@@ -11,6 +11,7 @@
  * @coversDefaultClass \Drupal\datastore_mysql_import\Storage\MySqlDatabaseTableFactory
  *
  * @group datastore_mysql_import
+ * @group kernel
  */
 class MySqlDatabaseTableFactoryTest extends KernelTestBase {
 
@@ -33,10 +34,8 @@ public function testFactoryServiceResourceException() {
   }
 
   public function testFactoryService() {
-    $identifier = 'identifier';
     $file_path = dirname(__FILE__, 4) . '/data/columnspaces.csv';
-    $datastore_resource = new DatastoreResource(
-      $identifier,
+    $datastore_resource = new DataResource(
       $file_path,
       'text/csv'
     );

From 0683cb7b2386a0486fb588947d9e58a3eed56642 Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Tue, 7 Jan 2025 15:17:32 -0500
Subject: [PATCH 11/23] Some code cleanup

---
 modules/common/src/DataResource.php                 | 13 ++++++++++---
 .../ResourceProcessor/DictionaryEnforcer.php        |  4 +++-
 modules/datastore/src/Storage/DatabaseTable.php     |  4 +---
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/modules/common/src/DataResource.php b/modules/common/src/DataResource.php
index c71cd866b0..72580ffd22 100644
--- a/modules/common/src/DataResource.php
+++ b/modules/common/src/DataResource.php
@@ -135,7 +135,8 @@ public function __construct($file_path, $mimeType, $perspective = self::DEFAULT_
    * @return \Drupal\common\DataResource
    *   DataResource object.
    *
-   * @deprecated in dkan:2.17.1 and is removed from dkan:2.20.0. Use DataResource::createFromEntity() instead.
+   * @deprecated in dkan:8.x-2.17 and is removed from dkan:8.x-2.21. Use
+   *   DataResource::createFromEntity() instead.
    * @see https://github.com/GetDKAN/dkan/pull/4027
    */
   public static function createFromRecord(object $record): DataResource {
@@ -236,7 +237,10 @@ public function changeMimeType($newMimeType) {
    * @return \Drupal\datastore\DatastoreResource
    *   Datastore Resource.
    *
-   * @deprecated
+   * @deprecated in dkan:8.x-2.20 and is removed from dkan:8.x-2.21. Use storage
+   *   classes like DatabaseTable::getTableName() to determine correct table
+   *   names, and pass true to ::getFilePath to get the resolved URL.
+   * @see https://github.com/GetDKAN/dkan/pull/4372
    */
   public function getDatastoreResource(): DatastoreResource {
     return new DatastoreResource(
@@ -332,7 +336,10 @@ public function getUniqueIdentifierNoPerspective(): string {
   /**
    * Retrieve datastore table name for resource.
    *
-   * @deprecated
+   * @deprecated in dkan:8.x-2.20 and is removed from dkan:8.x-2.21. Use storage
+   *  classes like DatabaseTable::getTableName() to determine correct table
+   *  names.
+   * @see https://github.com/GetDKAN/dkan/pull/4372
    */
   public function getTableName() {
     return 'datastore_' . md5($this->getUniqueIdentifier());
diff --git a/modules/datastore/src/Service/ResourceProcessor/DictionaryEnforcer.php b/modules/datastore/src/Service/ResourceProcessor/DictionaryEnforcer.php
index 3bb3dfc8b1..47e193dd4d 100644
--- a/modules/datastore/src/Service/ResourceProcessor/DictionaryEnforcer.php
+++ b/modules/datastore/src/Service/ResourceProcessor/DictionaryEnforcer.php
@@ -60,6 +60,8 @@ class DictionaryEnforcer implements ResourceProcessorInterface {
    *   The metastore service.
    * @param \Drupal\metastore\DataDictionary\DataDictionaryDiscoveryInterface $data_dictionary_discovery
    *   The data-dictionary discovery service.
+   * @param \Drupal\datastore\Storage\DatabaseTableFactory $table_factory
+   *   The datastore database table factory service.
    */
   public function __construct(
     AlterTableQueryBuilderInterface $alter_table_query_builder,
@@ -142,7 +144,7 @@ public function applyDictionary(RootedJsonData $dictionary, string $datastore_ta
    * @return array|null
    *   An array of dictionary fields or null if no dictionary is in use.
    */
-  public function returnDataDictionaryFields(string $identifier = NULL): ?array {
+  public function returnDataDictionaryFields(?string $identifier = NULL): ?array {
     // Get data dictionary mode.
     $dd_mode = $this->dataDictionaryDiscovery->getDataDictionaryMode();
     // Get data dictionary info.
diff --git a/modules/datastore/src/Storage/DatabaseTable.php b/modules/datastore/src/Storage/DatabaseTable.php
index 3536a32eb1..ee8a57597f 100755
--- a/modules/datastore/src/Storage/DatabaseTable.php
+++ b/modules/datastore/src/Storage/DatabaseTable.php
@@ -72,8 +72,6 @@ public function getSummary() {
   }
 
   /**
-   * Inherited.
-   *
    * {@inheritdoc}
    */
   #[\ReturnTypeWillChange]
@@ -97,7 +95,7 @@ public function getTableName() {
   /**
    * Protected.
    */
-  protected function prepareData(string $data, string $id = NULL): array {
+  protected function prepareData(string $data, ?string $id = NULL): array {
     $decoded = json_decode($data);
     if ($decoded === NULL) {
       $this->logger->error('Error decoding id:@id, data: @data.', [

From b34bd98306dab30fac65923f1169dc2040c829c6 Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Tue, 7 Jan 2025 15:20:56 -0500
Subject: [PATCH 12/23] More code cleanup

---
 modules/common/src/Storage/AbstractDatabaseTable.php   | 9 ++-------
 modules/datastore/src/Storage/DatabaseTable.php        | 1 -
 modules/datastore/src/Storage/DatabaseTableFactory.php | 2 +-
 3 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/modules/common/src/Storage/AbstractDatabaseTable.php b/modules/common/src/Storage/AbstractDatabaseTable.php
index e9268fc59c..f7bd4e5b25 100644
--- a/modules/common/src/Storage/AbstractDatabaseTable.php
+++ b/modules/common/src/Storage/AbstractDatabaseTable.php
@@ -20,11 +20,6 @@ abstract class AbstractDatabaseTable implements DatabaseTableInterface {
    */
   const EVENT_TABLE_CREATE = 'dkan_common_table_create';
 
-  /**
-   * Real table name.
-   */
-  protected string $tableName;
-
   /**
    * A schema. Should be a drupal schema array.
    *
@@ -45,7 +40,7 @@ abstract class AbstractDatabaseTable implements DatabaseTableInterface {
    * Transform the string data given into what should be use by the insert
    * query.
    */
-  abstract protected function prepareData(string $data, string $id = NULL): array;
+  abstract protected function prepareData(string $data, ?string $id = NULL): array;
 
   /**
    * Get the primary key used in the table.
@@ -110,7 +105,7 @@ public function retrieveAll(): array {
   /**
    * Store data.
    */
-  public function store($data, string $id = NULL): string {
+  public function store($data, ?string $id = NULL): string {
     $this->setTable();
 
     $existing = (isset($id)) ? $this->retrieve($id) : NULL;
diff --git a/modules/datastore/src/Storage/DatabaseTable.php b/modules/datastore/src/Storage/DatabaseTable.php
index ee8a57597f..6e7a93c639 100755
--- a/modules/datastore/src/Storage/DatabaseTable.php
+++ b/modules/datastore/src/Storage/DatabaseTable.php
@@ -49,7 +49,6 @@ public function __construct(
 
     if ($this->tableExist($this->getTableName())) {
       $this->setSchemaFromTable();
-      $this->tableName = $this->getTableName();
     }
   }
 
diff --git a/modules/datastore/src/Storage/DatabaseTableFactory.php b/modules/datastore/src/Storage/DatabaseTableFactory.php
index a98f88e152..9ade6a0e24 100644
--- a/modules/datastore/src/Storage/DatabaseTableFactory.php
+++ b/modules/datastore/src/Storage/DatabaseTableFactory.php
@@ -28,7 +28,7 @@ class DatabaseTableFactory implements FactoryInterface {
    */
   public function __construct(
     Connection $connection,
-    LoggerInterface $loggerChannel
+    LoggerInterface $loggerChannel,
   ) {
     $this->connection = $connection;
     $this->logger = $loggerChannel;

From 765af01f92fe85aaf77755a978a93e1ab6a642c7 Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Tue, 7 Jan 2025 15:22:33 -0500
Subject: [PATCH 13/23] Fix param

---
 .../src/Service/ResourceProcessor/DictionaryEnforcer.php        | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/modules/datastore/src/Service/ResourceProcessor/DictionaryEnforcer.php b/modules/datastore/src/Service/ResourceProcessor/DictionaryEnforcer.php
index 47e193dd4d..494b56a569 100644
--- a/modules/datastore/src/Service/ResourceProcessor/DictionaryEnforcer.php
+++ b/modules/datastore/src/Service/ResourceProcessor/DictionaryEnforcer.php
@@ -138,7 +138,7 @@ public function applyDictionary(RootedJsonData $dictionary, string $datastore_ta
   /**
    * Returning data dictionary fields from schema.
    *
-   * @param string $identifier
+   * @param string|null $identifier
    *   A resource's identifier. Used when in reference mode.
    *
    * @return array|null

From ea833203470a3362bc446187818970ab3aba9507 Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Tue, 7 Jan 2025 15:45:39 -0500
Subject: [PATCH 14/23] Deprecate DatastoreResource

---
 modules/datastore/src/DatastoreResource.php | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/modules/datastore/src/DatastoreResource.php b/modules/datastore/src/DatastoreResource.php
index 13ca348f92..9239e9474d 100644
--- a/modules/datastore/src/DatastoreResource.php
+++ b/modules/datastore/src/DatastoreResource.php
@@ -5,9 +5,9 @@
 /**
  * Basic datastore resource class.
  *
- * Always generate this object using DataResource::getDatastoreResource().
- *
- * @see \Drupal\common\DataResource::getDatastoreResource()
+ * @deprecated in dkan:8.x-2.20 and is removed from dkan:8.x-2.21. Use
+ *   \Drupal\common\DataResource instead.
+ * @see https://github.com/GetDKAN/dkan/pull/4372
  */
 class DatastoreResource implements \JsonSerializable {
 

From da40af0dd6a6047f410361f1152002a8a23bb916 Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Tue, 7 Jan 2025 15:58:26 -0500
Subject: [PATCH 15/23] Remove debug code

---
 modules/json_form_widget/tests/src/Unit/StringHelperTest.php | 1 -
 1 file changed, 1 deletion(-)

diff --git a/modules/json_form_widget/tests/src/Unit/StringHelperTest.php b/modules/json_form_widget/tests/src/Unit/StringHelperTest.php
index 8039ee8417..2edf8f451b 100644
--- a/modules/json_form_widget/tests/src/Unit/StringHelperTest.php
+++ b/modules/json_form_widget/tests/src/Unit/StringHelperTest.php
@@ -102,7 +102,6 @@ public function testHandleStringElement() {
     $data = 'mailto:john@doe.com';
 
     $result = $string_helper->handleStringElement($definition, $data);
-    print_r($result['#default_value']);
     $this->assertEquals('john@doe.com', $result['#default_value']);
   }
 

From ef080e99b5c0e9327143a9ad836034d62ba1400c Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Wed, 8 Jan 2025 12:11:57 -0500
Subject: [PATCH 16/23] Improve deprecation notices

---
 modules/datastore/src/DatastoreResource.php | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/modules/datastore/src/DatastoreResource.php b/modules/datastore/src/DatastoreResource.php
index 9239e9474d..439b114a54 100644
--- a/modules/datastore/src/DatastoreResource.php
+++ b/modules/datastore/src/DatastoreResource.php
@@ -44,7 +44,9 @@ public function __construct($id, $file_path, $mime_type) {
   /**
    * Get the resource ID.
    *
-   * Note: duplicates Drupal\common\DataResource::getUniqueIdentifier().
+   * @deprecated in dkan:8.x-2.20 and is removed from dkan:8.x-2.21. Use
+   *   \Drupal\common\DataResource::getUniqueIdentifier() instead.
+   * @see https://github.com/GetDKAN/dkan/pull/4372
    */
   public function getId(): string {
     return $this->id;
@@ -53,7 +55,12 @@ public function getId(): string {
   /**
    * Get the file path.
    *
-   * Note: duplicates Drupal\common\DataResource::getFilePath(TRUE).
+   * @return string
+   *   The file path.
+   *
+   * @deprecated in dkan:8.x-2.20 and is removed from dkan:8.x-2.21. Use
+   *   \Drupal\common\DataResource::getUniqueIdentifier() instead.
+   * @see https://github.com/GetDKAN/dkan/pull/4372
    */
   public function getFilePath(): string {
     return $this->filePath;
@@ -62,7 +69,12 @@ public function getFilePath(): string {
   /**
    * Get the mimeType.
    *
-   * Note: duplicates Drupal\common\DataResource::getMimeType().
+   * @return string
+   *   The mimeType.
+   *
+   * @deprecated in dkan:8.x-2.20 and is removed from dkan:8.x-2.21. Use
+   *   \Drupal\common\DataResource::getMimeType() instead.
+   * @see https://github.com/GetDKAN/dkan/pull/4372
    */
   public function getMimeType(): string {
     return $this->mimeType;

From b359d59f23f33e53642fe71abe2cc5ccb46d0d30 Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Wed, 8 Jan 2025 12:12:14 -0500
Subject: [PATCH 17/23] Unused use statements

---
 modules/datastore/tests/src/Kernel/Storage/DatabaseTableTest.php | 1 -
 .../datastore/tests/src/Unit/Controller/QueryControllerTest.php  | 1 -
 2 files changed, 2 deletions(-)

diff --git a/modules/datastore/tests/src/Kernel/Storage/DatabaseTableTest.php b/modules/datastore/tests/src/Kernel/Storage/DatabaseTableTest.php
index 1c9dda48bf..71994466a1 100644
--- a/modules/datastore/tests/src/Kernel/Storage/DatabaseTableTest.php
+++ b/modules/datastore/tests/src/Kernel/Storage/DatabaseTableTest.php
@@ -5,7 +5,6 @@
 use ColinODell\PsrTestLogger\TestLogger;
 use Drupal\common\DataResource;
 use Drupal\common\Storage\ImportedItemInterface;
-use Drupal\datastore\DatastoreResource;
 use Drupal\datastore\Plugin\QueueWorker\ImportJob;
 use Drupal\datastore\Service\Factory\ImportServiceFactory;
 use Drupal\datastore\Storage\DatabaseTable;
diff --git a/modules/datastore/tests/src/Unit/Controller/QueryControllerTest.php b/modules/datastore/tests/src/Unit/Controller/QueryControllerTest.php
index 3427fc231b..4f3c9a274e 100644
--- a/modules/datastore/tests/src/Unit/Controller/QueryControllerTest.php
+++ b/modules/datastore/tests/src/Unit/Controller/QueryControllerTest.php
@@ -9,7 +9,6 @@
 use Drupal\common\DatasetInfo;
 use Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher;
 use Drupal\datastore\Controller\QueryController;
-use Drupal\datastore\DatastoreResource;
 use Drupal\datastore\DatastoreService;
 use Drupal\datastore\Service\Query;
 use Drupal\datastore\Storage\SqliteDatabaseTable;

From a140f2a745f4a17a3d789e961f9b93a943be0c60 Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Wed, 8 Jan 2025 12:12:25 -0500
Subject: [PATCH 18/23] Fix ImportJobTest

---
 .../Unit/Plugin/QueueWorker/ImportJobTest.php | 56 +++++++++++++------
 1 file changed, 39 insertions(+), 17 deletions(-)

diff --git a/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php b/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php
index 53c0c5bdd2..7dbd0f424a 100644
--- a/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php
+++ b/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php
@@ -5,11 +5,18 @@
 use Contracts\Mock\Storage\Memory;
 use CsvParser\Parser\Csv;
 use CsvParser\Parser\ParserInterface;
+use Drupal\common\DataResource;
 use Drupal\common\Storage\DatabaseTableInterface;
-use Drupal\datastore\DatastoreResource;
+use Drupal\Component\DependencyInjection\Container;
+use Drupal\Core\StreamWrapper\StreamWrapperInterface;
+use Drupal\Core\StreamWrapper\StreamWrapperManager;
 use Drupal\datastore\Plugin\QueueWorker\ImportJob;
+use MockChain\Chain;
+use MockChain\Options;
 use PHPUnit\Framework\TestCase;
 use Procrastinator\Result;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
 
 /**
  * Unit tests for Importer class.
@@ -38,6 +45,17 @@ protected function setUp(): void {
     parent::setUp();
     $this->database = new TestMemStorage();
     $this->assertTrue($this->database instanceof DatabaseTableInterface);
+
+    $options = (new Options())
+      ->add('stream_wrapper_manager', StreamWrapperManager::class)
+      ->add('request_stack', RequestStack::class)
+      ->index(0);
+    $container = (new Chain($this))
+      ->add(Container::class, 'get', $options)
+      ->add(StreamWrapperManager::class, 'getViaUri', StreamWrapperInterface::class)
+      ->add(RequestStack::class, 'getCurrentRequest', Request::class)
+      ->add(Request::class, 'getHost', 'web');
+    \Drupal::setContainer($container->getMock());
   }
 
   protected function tearDown(): void {
@@ -46,24 +64,29 @@ protected function tearDown(): void {
   }
 
   /**
+   * Get an ImportJob object.
+   *
+   * @param \Drupal\common\DataResource $resource
+   *   DataResource object.
    *
+   * @return \Drupal\datastore\Plugin\QueueWorker\ImportJob
+   *   ImportJob object.
    */
-  private function getImportJob(DatastoreResource $resource): ImportJob {
+  private function getImportJob(DataResource $resource): ImportJob {
     $storage = new Memory();
     $config = [
       'resource' => $resource,
       'storage' => $this->database,
       'parser' => Csv::getParser(),
     ];
-    return ImportJob::get('1', $storage, $config);
+    return ImportJob::get($resource->getUniqueIdentifier(), $storage, $config);
   }
 
   /**
    *
    */
   public function testBasics() {
-    $resource = new DatastoreResource(1, __DIR__ . '/../../../../data/countries.csv', 'text/csv');
-    $this->assertEquals(1, $resource->getID());
+    $resource = new DataResource(__DIR__ . '/../../../../data/countries.csv', 'text/csv');
 
     $import_job = $this->getImportJob($resource);
 
@@ -95,7 +118,7 @@ public function testBasics() {
    *
    */
   public function testFileNotFound() {
-    $resource = new DatastoreResource(1, __DIR__ . '/../../../../data/non-existent.csv', 'text/csv');
+    $resource = new DataResource(__DIR__ . '/../../../../data/non-existent.csv', 'text/csv');
     $datastore = $this->getImportJob($resource);
     $datastore->run();
 
@@ -106,7 +129,7 @@ public function testFileNotFound() {
    *
    */
   public function testNonTextFile() {
-    $resource = new DatastoreResource(1, __DIR__ . '/../../../../data/non-text.csv', 'text/csv');
+    $resource = new DataResource(__DIR__ . '/../../../../data/non-text.csv', 'text/csv');
     $datastore = $this->getImportJob($resource);
     $datastore->run();
 
@@ -117,7 +140,7 @@ public function testNonTextFile() {
    *
    */
   public function testDuplicateHeaders() {
-    $resource = new DatastoreResource(1, __DIR__ . '/../../../../data/duplicate-headers.csv', 'text/csv');
+    $resource = new DataResource(__DIR__ . '/../../../../data/duplicate-headers.csv', 'text/csv');
     $datastore = $this->getImportJob($resource);
     $datastore->run();
 
@@ -130,7 +153,7 @@ public function testDuplicateHeaders() {
    *
    */
   public function testLongColumnName() {
-    $resource = new DatastoreResource(1, __DIR__ . '/../../../../data/longcolumn.csv', 'text/csv');
+    $resource = new DataResource(__DIR__ . '/../../../../data/longcolumn.csv', 'text/csv');
     $datastore = $this->getImportJob($resource);
     $truncatedLongFieldName = 'extra_long_column_name_with_tons_of_characters_that_will_ne_e872';
 
@@ -149,7 +172,7 @@ public function testLongColumnName() {
    *
    */
   public function testColumnNameSpaces() {
-    $resource = new DatastoreResource(1, __DIR__ . '/../../../../data/columnspaces.csv', 'text/csv');
+    $resource = new DataResource(__DIR__ . '/../../../../data/columnspaces.csv', 'text/csv');
     $datastore = $this->getImportJob($resource);
     $noMoreSpaces = 'column_name_with_spaces_in_it';
 
@@ -168,8 +191,7 @@ public function testColumnNameSpaces() {
    */
   public function testSerialization() {
     $timeLimit = 40;
-    $resource = new DatastoreResource(1, __DIR__ . '/../../../../data/countries.csv', 'text/csv');
-    $this->assertEquals(1, $resource->getID());
+    $resource = new DataResource(__DIR__ . '/../../../../data/countries.csv', 'text/csv');
 
     $datastore = $this->getImportJob($resource);
     $datastore->setTimeLimit($timeLimit);
@@ -186,7 +208,7 @@ public function testSerialization() {
    * Test whether a potential multi-batch import works correctly.
    */
   public function testLargeImport() {
-    $resource = new DatastoreResource(1, __DIR__ . '/../../../../data/Bike_Lane.csv', 'text/csv');
+    $resource = new DataResource(__DIR__ . '/../../../../data/Bike_Lane.csv', 'text/csv');
 
     $storage = new Memory();
 
@@ -223,7 +245,7 @@ public function testLargeImport() {
    */
   public function testMultiplePasses() {
     $this->markTestIncomplete('This does not always use more than one pass.');
-    $resource = new DatastoreResource(1, __DIR__ . '/../../../../data/Bike_Lane.csv', 'text/csv');
+    $resource = new DataResource(__DIR__ . '/../../../../data/Bike_Lane.csv', 'text/csv');
 
     $storage = new Memory();
 
@@ -265,9 +287,9 @@ public function testMultiplePasses() {
    */
   public function testBadStorage() {
     $this->expectExceptionMessage('Storage must be an instance of ' . DatabaseTableInterface::class);
-    $resource = new DatastoreResource(1, __DIR__ . '/../../../../data/countries.csZv', 'text/csv');
+    $resource = new DataResource(__DIR__ . '/../../../../data/countries.csZv', 'text/csv');
 
-    ImportJob::get('1', new Memory(), [
+    ImportJob::get($resource->getUniqueIdentifier(), new Memory(), [
       'resource' => $resource,
       'storage' => new TestMemStorageBad(),
       'parser' => Csv::getParser(),
@@ -279,7 +301,7 @@ public function testBadStorage() {
    */
   public function testNonStorage() {
     $this->expectExceptionMessage('Storage must be an instance of Drupal\common\Storage\DatabaseTableInterface');
-    $resource = new DatastoreResource(1, __DIR__ . '/../../../../data/countries.csv', 'text/csv');
+    $resource = new DataResource(__DIR__ . '/../../../../data/countries.csv', 'text/csv');
     ImportJob::get('1', new Memory(), [
       'resource' => $resource,
       'storage' => new class() {

From 731cb33434ac37b978d4020c436937f4797698a5 Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Wed, 8 Jan 2025 12:14:03 -0500
Subject: [PATCH 19/23] Fix ImportInfoTest

---
 .../tests/src/Unit/Service/Info/ImportInfoTest.php          | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/modules/datastore/tests/src/Unit/Service/Info/ImportInfoTest.php b/modules/datastore/tests/src/Unit/Service/Info/ImportInfoTest.php
index f1b65030d1..d7b93daebb 100644
--- a/modules/datastore/tests/src/Unit/Service/Info/ImportInfoTest.php
+++ b/modules/datastore/tests/src/Unit/Service/Info/ImportInfoTest.php
@@ -4,7 +4,7 @@
 
 use Contracts\Mock\Storage\Memory;
 use CsvParser\Parser\Csv;
-use Drupal\datastore\DatastoreResource;
+use Drupal\common\DataResource;
 use Drupal\datastore\Plugin\QueueWorker\ImportJob;
 use Drupal\datastore\Service\Info\ImportInfo;
 use Drupal\Tests\datastore\Unit\Plugin\QueueWorker\TestMemStorage;
@@ -56,7 +56,7 @@ public function testGetBytesProcessedFileFetcher() {
     // Make a FileFetcher object.
     $storage = new Memory();
     $config = [
-      "resource" => (new DatastoreResource('id', 'path', 'mime')),
+      "resource" => (new DataResource('path', 'mime')),
       "storage" => new TestMemStorage(),
       "parser" => Csv::getParser(),
       "filePath" => 'test',
@@ -87,7 +87,7 @@ public function testGetBytesProcessedImportJob() {
     // Make an ImportJob object.
     $storage = new Memory();
     $config = [
-      "resource" => (new DatastoreResource('id', 'path', 'mime')),
+      "resource" => (new DataResource('path', 'mime')),
       "storage" => new TestMemStorage(),
       "parser" => Csv::getParser(),
     ];

From 315277f98ac804077f9ac5f1a4605ce1ca3eb6ed Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Wed, 8 Jan 2025 12:15:54 -0500
Subject: [PATCH 20/23] Fix DatabaseTableFactoryTest

---
 .../tests/src/Unit/Storage/DatabaseTableFactoryTest.php     | 6 +++---
 .../datastore/tests/src/Unit/Storage/DatabaseTableTest.php  | 1 -
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/modules/datastore/tests/src/Unit/Storage/DatabaseTableFactoryTest.php b/modules/datastore/tests/src/Unit/Storage/DatabaseTableFactoryTest.php
index 614d60e5f7..d4f7e220bd 100644
--- a/modules/datastore/tests/src/Unit/Storage/DatabaseTableFactoryTest.php
+++ b/modules/datastore/tests/src/Unit/Storage/DatabaseTableFactoryTest.php
@@ -3,7 +3,7 @@
 namespace Drupal\Tests\datastore\Unit\Storage;
 
 use Drupal\Core\Database\Connection;
-use Drupal\datastore\DatastoreResource;
+use Drupal\common\DataResource;
 use Drupal\datastore\Storage\DatabaseTable;
 use Drupal\datastore\Storage\DatabaseTableFactory;
 use MockChain\Chain;
@@ -39,8 +39,8 @@ public function test() {
 
     $factory->method("getDatabaseTable")->willReturn($databaseTable);
 
-    $resource = new DatastoreResource("blah", "", "text/csv");
-    $object = $factory->getInstance($resource->getId(), ['resource' => $resource]);
+    $resource = new DataResource("", "text/csv");
+    $object = $factory->getInstance($resource->getUniqueIdentifier(), ['resource' => $resource]);
     $this->assertTrue($object instanceof DatabaseTable);
   }
 
diff --git a/modules/datastore/tests/src/Unit/Storage/DatabaseTableTest.php b/modules/datastore/tests/src/Unit/Storage/DatabaseTableTest.php
index 4a87ce7148..269a7b527d 100644
--- a/modules/datastore/tests/src/Unit/Storage/DatabaseTableTest.php
+++ b/modules/datastore/tests/src/Unit/Storage/DatabaseTableTest.php
@@ -9,7 +9,6 @@
 use Drupal\Core\Database\Query\Select;
 use Drupal\Core\Database\StatementWrapper;
 use Drupal\common\Storage\Query;
-use Drupal\datastore\DatastoreResource;
 use Drupal\datastore\Storage\DatabaseTable;
 use Drupal\mysql\Driver\Database\mysql\Schema;
 use MockChain\Chain;

From f7aa9bd07cb03220c7535b28b44403e0a12bde89 Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Wed, 8 Jan 2025 16:50:11 -0500
Subject: [PATCH 21/23] RestoreDataResourceTest

---
 .../common/tests/src/Unit/DataResourceTest.php    | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/modules/common/tests/src/Unit/DataResourceTest.php b/modules/common/tests/src/Unit/DataResourceTest.php
index a2413f12fc..078907b554 100644
--- a/modules/common/tests/src/Unit/DataResourceTest.php
+++ b/modules/common/tests/src/Unit/DataResourceTest.php
@@ -18,6 +18,21 @@
  */
 class DataResourceTest extends TestCase {
 
+  /**
+   * Test getTableName().
+   */
+  public function testGetTableName() {
+
+    $resource = new DataResource(
+      '/foo/bar',
+      'txt',
+      DataResource::DEFAULT_SOURCE_PERSPECTIVE
+    );
+    $tableName = $resource->getTableName();
+
+    $this->assertStringStartsWith('datastore_', $tableName);
+  }
+
   /**
    * Test getFolder().
    */

From 2594e68fb5e58d9f5062bb82b9ad301e4cdd42bb Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Wed, 8 Jan 2025 16:56:25 -0500
Subject: [PATCH 22/23] Better docs for DatabaseTableFactory

---
 .../src/Storage/DatabaseTableFactory.php       | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/modules/datastore/src/Storage/DatabaseTableFactory.php b/modules/datastore/src/Storage/DatabaseTableFactory.php
index 9ade6a0e24..bbfe15bfd6 100644
--- a/modules/datastore/src/Storage/DatabaseTableFactory.php
+++ b/modules/datastore/src/Storage/DatabaseTableFactory.php
@@ -35,9 +35,15 @@ public function __construct(
   }
 
   /**
-   * Inherited.
+   * Get a DatabaseTable instance.
    *
-   * @inheritdoc
+   * @param string $identifier
+   *   Some way to discern between different instances of a class.
+   * @param array $config
+   *   Must contain a 'resource' key, which is a DataResource object.
+   *
+   * @return \Drupal\datastore\Storage\DatabaseTable
+   *   A DatabaseTable object.
    */
   public function getInstance(string $identifier, array $config = []) {
     if (!isset($config['resource'])) {
@@ -50,7 +56,13 @@ public function getInstance(string $identifier, array $config = []) {
   }
 
   /**
-   * Protected.
+   * Get a DatabaseTable object from a DataResource object.
+   *
+   * @param \Drupal\common\DataResource $resource
+   *   A resource.
+   *
+   * @return \Drupal\datastore\Storage\DatabaseTable
+   *   A DatabaseTable object.
    */
   protected function getDatabaseTable($resource) {
     return new DatabaseTable($this->connection, $resource, $this->logger);

From 0a2169ebb9ae2fe3a19498f2c89432b6428a76c3 Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Wed, 8 Jan 2025 16:59:19 -0500
Subject: [PATCH 23/23] Add type hint

---
 modules/datastore/src/Storage/DatabaseTableFactory.php | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/modules/datastore/src/Storage/DatabaseTableFactory.php b/modules/datastore/src/Storage/DatabaseTableFactory.php
index bbfe15bfd6..c7bc0098ee 100644
--- a/modules/datastore/src/Storage/DatabaseTableFactory.php
+++ b/modules/datastore/src/Storage/DatabaseTableFactory.php
@@ -3,6 +3,7 @@
 namespace Drupal\datastore\Storage;
 
 use Contracts\FactoryInterface;
+use Drupal\common\DataResource;
 use Drupal\Core\Database\Connection;
 use Psr\Log\LoggerInterface;
 
@@ -64,7 +65,7 @@ public function getInstance(string $identifier, array $config = []) {
    * @return \Drupal\datastore\Storage\DatabaseTable
    *   A DatabaseTable object.
    */
-  protected function getDatabaseTable($resource) {
+  protected function getDatabaseTable(DataResource $resource) {
     return new DatabaseTable($this->connection, $resource, $this->logger);
   }