From 12a29ad601b441c585439fb6db46986c0a2021ce Mon Sep 17 00:00:00 2001
From: Paul Mitchum <paul@mile23.com>
Date: Mon, 27 Mar 2023 12:33:17 -0700
Subject: [PATCH 1/9] removed all the hydration unrelated to procrastinator

---
 modules/common/src/DataResource.php           | 31 +++++++++++++++-
 modules/datastore/src/DatastoreResource.php   | 37 +++----------------
 .../datastore/src/Storage/DatabaseTable.php   | 10 -----
 .../Unit/Plugin/QueueWorker/ImportJobTest.php |  3 --
 .../Plugin/QueueWorker/TestMemStorage.php     | 12 ------
 modules/metastore/src/ResourceMapper.php      | 13 +++++--
 .../src/Storage/MetastoreStorageInterface.php |  2 +-
 .../src/Unit/Reference/ReferencerTest.php     |  3 +-
 tests/src/Functional/DatasetTest.php          |  1 +
 9 files changed, 49 insertions(+), 63 deletions(-)

diff --git a/modules/common/src/DataResource.php b/modules/common/src/DataResource.php
index ff31e3f63c..8ef5d24ad0 100644
--- a/modules/common/src/DataResource.php
+++ b/modules/common/src/DataResource.php
@@ -3,7 +3,6 @@
 namespace Drupal\common;
 
 use Drupal\datastore\DatastoreResource;
-use Procrastinator\HydratableTrait;
 use Procrastinator\JsonSerializeTrait;
 
 /**
@@ -28,7 +27,7 @@
  * @todo Refactor as service.
  */
 class DataResource implements \JsonSerializable {
-  use HydratableTrait, JsonSerializeTrait;
+  use JsonSerializeTrait;
 
   const DEFAULT_SOURCE_PERSPECTIVE = 'source';
 
@@ -78,6 +77,13 @@ class DataResource implements \JsonSerializable {
 
   /**
    * Constructor.
+   *
+   * @param string $file_path
+   *   Path to the file.
+   * @param string $mimeType
+   *   File mime type.
+   * @param string $perspective
+   *   Can be one of "local_file", "local_url", or "source".
    */
   public function __construct($file_path, $mimeType, $perspective = self::DEFAULT_SOURCE_PERSPECTIVE) {
     // @todo generate UUID instead.
@@ -90,6 +96,27 @@ public function __construct($file_path, $mimeType, $perspective = self::DEFAULT_
     $this->checksum = NULL;
   }
 
+  /**
+   * Create a DataResource object from a database record.
+   *
+   * @param object $record
+   *   Data resource record from the database. Must contain these properties:
+   *   - filePath
+   *   - mimeType
+   *   - perspective
+   *   - version
+   *
+   * @return \Drupal\common\DataResource
+   */
+  public static function createFromRecord(object $record): DataResource {
+    $resource = new static($record->filePath, $record->mimeType, $record->perspective);
+    // MD5 of record's file path can differ from the MD5 generated in the
+    // constructor, so we have to explicitly set the identifier.
+    $resource->identifier = $record->identifier;
+    $resource->version = $record->version;
+    return $resource;
+  }
+
   /**
    * Create a new version.
    *
diff --git a/modules/datastore/src/DatastoreResource.php b/modules/datastore/src/DatastoreResource.php
index 98ef237909..fc04b9c20c 100644
--- a/modules/datastore/src/DatastoreResource.php
+++ b/modules/datastore/src/DatastoreResource.php
@@ -40,21 +40,21 @@ public function __construct($id, $file_path, $mime_type) {
   /**
    * Get the resource ID.
    */
-  public function getId() {
+  public function getId(): string {
     return $this->id;
   }
 
   /**
    * Get the file path.
    */
-  public function getFilePath() {
+  public function getFilePath(): string {
     return $this->filePath;
   }
 
   /**
    * Get the mimeType.
    */
-  public function getMimeType() {
+  public function getMimeType(): string {
     return $this->mimeType;
   }
 
@@ -64,35 +64,10 @@ public function getMimeType() {
   #[\ReturnTypeWillChange]
   public function jsonSerialize() {
     return (object) [
-      'filePath' => $this->filePath,
-      'id' => $this->id,
-      'mimeType' => $this->mimeType,
+      'filePath' => $this->getFilePath(),
+      'id' => $this->getId(),
+      'mimeType' => $this->getMimeType(),
     ];
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public static function hydrate($json) {
-    $data = json_decode($json);
-    $reflector = new \ReflectionClass(self::class);
-    $object = $reflector->newInstanceWithoutConstructor();
-
-    $reflector = new \ReflectionClass($object);
-
-    $p = $reflector->getProperty('filePath');
-    $p->setAccessible(TRUE);
-    $p->setValue($object, $data->filePath);
-
-    $p = $reflector->getProperty('id');
-    $p->setAccessible(TRUE);
-    $p->setValue($object, $data->id);
-
-    $p = $reflector->getProperty('mimeType');
-    $p->setAccessible(TRUE);
-    $p->setValue($object, $data->mimeType);
-
-    return $object;
-  }
-
 }
diff --git a/modules/datastore/src/Storage/DatabaseTable.php b/modules/datastore/src/Storage/DatabaseTable.php
index ebdd153a42..9615ff698e 100755
--- a/modules/datastore/src/Storage/DatabaseTable.php
+++ b/modules/datastore/src/Storage/DatabaseTable.php
@@ -70,16 +70,6 @@ public function jsonSerialize() {
     return (object) ['resource' => $this->resource];
   }
 
-  /**
-   * Hydrate.
-   */
-  public static function hydrate(string $json) {
-    $data = json_decode($json);
-    $resource = DatastoreResource::hydrate(json_encode($data->resource));
-
-    return new DatabaseTable(\Drupal::service('database'), $resource);
-  }
-
   /**
    * Get the full name of datastore db table.
    *
diff --git a/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php b/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php
index 5073538644..bbbe67497f 100644
--- a/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php
+++ b/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php
@@ -224,9 +224,6 @@ public function testBadStorage() {
       "storage" => new TestMemStorageBad(),
       "parser" => Csv::getParser(),
     ]);
-
-    $json = json_encode($importer);
-    ImportJob::hydrate($json);
   }
 
   /**
diff --git a/modules/datastore/tests/src/Unit/Plugin/QueueWorker/TestMemStorage.php b/modules/datastore/tests/src/Unit/Plugin/QueueWorker/TestMemStorage.php
index 9744c305ab..a6e65c0d47 100644
--- a/modules/datastore/tests/src/Unit/Plugin/QueueWorker/TestMemStorage.php
+++ b/modules/datastore/tests/src/Unit/Plugin/QueueWorker/TestMemStorage.php
@@ -79,18 +79,6 @@ public function jsonSerialize()
         return (object) ['storage' => $this->storage];
     }
 
-    public static function hydrate(string $json)
-    {
-        $class = new \ReflectionClass(self::class);
-        $instance = $class->newInstanceWithoutConstructor();
-
-        $property = $class->getParentClass()->getProperty('storage');
-        $property->setAccessible(true);
-        $property->setValue($instance, (array) (json_decode($json))->storage);
-
-        return $instance;
-    }
-
     /**
      * Clean up and set the schema for SQL storage.
      *
diff --git a/modules/metastore/src/ResourceMapper.php b/modules/metastore/src/ResourceMapper.php
index c40187f4d9..265e58e1d5 100644
--- a/modules/metastore/src/ResourceMapper.php
+++ b/modules/metastore/src/ResourceMapper.php
@@ -8,17 +8,21 @@
 use Drupal\common\EventDispatcherTrait;
 use Drupal\datastore\Service\ResourceLocalizer;
 use Drupal\metastore\Exception\AlreadyRegistered;
+use Drupal\metastore\NodeWrapper\Data;
 
 /**
  * Map resource URLs to local files.
  */
 class ResourceMapper {
+
   use EventDispatcherTrait;
 
   const EVENT_REGISTRATION = 'dkan_metastore_resource_mapper_registration';
+
   const EVENT_RESOURCE_MAPPER_PRE_REMOVE_SOURCE = 'dkan_metastore_pre_remove_source';
 
   const DEREFERENCE_NO = 0;
+
   const DEREFERENCE_YES = 1;
 
   /**
@@ -58,7 +62,7 @@ public static function newRevision() {
    * @todo the Resource class currently lives in datastore, we should move it
    * to a more neutral place.
    */
-  public function register(DataResource $resource) : bool {
+  public function register(DataResource $resource): bool {
     $this->filePathExists($resource->getFilePath());
     $this->store->store(json_encode($resource));
     $this->dispatchEvent(self::EVENT_REGISTRATION, $resource);
@@ -133,7 +137,10 @@ protected function validateNewVersion(DataResource $resource) {
    */
   public function get(string $identifier, $perspective = DataResource::DEFAULT_SOURCE_PERSPECTIVE, $version = NULL): ?DataResource {
     $data = $this->getFull($identifier, $perspective, $version);
-    return ($data != FALSE) ? DataResource::hydrate(json_encode($data)) : NULL;
+    if ($data !== FALSE) {
+      return DataResource::createFromRecord($data);
+    }
+    return NULL;
   }
 
   /**
@@ -238,7 +245,7 @@ public function filePathExists($filePath) {
   /**
    * Private.
    */
-  private function exists($identifier, $perspective, $version = NULL) : bool {
+  private function exists($identifier, $perspective, $version = NULL): bool {
     $item = $this->get($identifier, $perspective, $version);
     return isset($item);
   }
diff --git a/modules/metastore/src/Storage/MetastoreStorageInterface.php b/modules/metastore/src/Storage/MetastoreStorageInterface.php
index a7ca14ed5b..9a926cdc16 100644
--- a/modules/metastore/src/Storage/MetastoreStorageInterface.php
+++ b/modules/metastore/src/Storage/MetastoreStorageInterface.php
@@ -26,7 +26,7 @@ public function count(bool $unpublished = FALSE): int;
    * @param bool $published
    *   Whether to retrieve the published revision of the metadata.
    *
-   * @return string|HydratableInterface
+   * @return string|null
    *   The data or null if no data could be retrieved.
    *
    * @throws \Drupal\metastore\Exception\MissingObjectException
diff --git a/modules/metastore/tests/src/Unit/Reference/ReferencerTest.php b/modules/metastore/tests/src/Unit/Reference/ReferencerTest.php
index 2b4871374f..64a7f8034c 100644
--- a/modules/metastore/tests/src/Unit/Reference/ReferencerTest.php
+++ b/modules/metastore/tests/src/Unit/Reference/ReferencerTest.php
@@ -275,7 +275,8 @@ public function testChangeMediaType() {
     }';
     $data = json_decode($json);
     $referencer->reference($data);
-    $storedResource = DataResource::hydrate($container_chain->getStoredInput('resource')[0]);
+    // @todo: Where does this stored input come from?
+    $storedResource = DataResource::createFromRecord(json_decode($container_chain->getStoredInput('resource')[0]));
     // A new resource should have been stored, with the mimetype set to text/csv
     $this->assertEquals('text/csv', $storedResource->getMimeType());
   }
diff --git a/tests/src/Functional/DatasetTest.php b/tests/src/Functional/DatasetTest.php
index 6bdb23ca6e..9a78fcb9c6 100644
--- a/tests/src/Functional/DatasetTest.php
+++ b/tests/src/Functional/DatasetTest.php
@@ -105,6 +105,7 @@ public function testResourcePurgeDraft() {
     $this->getMetastore()->publish('dataset', $id_1);
     $this->storeDatasetRunQueues($id_1, '1.3', ['1.csv', '5.csv'], 'put');
 
+    /** @var \Drupal\common\DatasetInfo $datasetInfo */
     $datasetInfo = \Drupal::service('dkan.common.dataset_info');
     $info = $datasetInfo->gather($id_1);
     $this->assertStringEndsWith('1.csv', $info['latest_revision']['distributions'][0]['file_path']);

From 14f3b23614c0cfab39e8692cfb3333cb6b337534 Mon Sep 17 00:00:00 2001
From: Paul Mitchum <paul@mile23.com>
Date: Thu, 30 Mar 2023 12:48:32 -0700
Subject: [PATCH 2/9] matrix test, php 8.1 composer, fix test

---
 .circleci/config.yml                          | 174 +++++++++++++-----
 composer.json                                 |  10 +-
 .../src/Unit/Reference/ReferencerTest.php     |  12 +-
 3 files changed, 140 insertions(+), 56 deletions(-)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index 42855d5bc6..e87141d135 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -1,4 +1,8 @@
-version: 2.1
+# Circle CI script to run a matrix of tests in DDev.
+
+version: '2.1'
+orbs:
+  docker: circleci/docker@2.2.0
 
 commands:
   install-ddev:
@@ -13,13 +17,21 @@ commands:
             curl https://apt.fury.io/drud/gpg.key | sudo apt-key add -
             echo "deb https://apt.fury.io/drud/ * *" | sudo tee -a /etc/apt/sources.list.d/ddev.list
             sudo apt update && sudo apt install -y ddev
+
   prepare_build:
-    # TODO: Figure out the best way to share this build between the various jobs.
+    # TODO: Currently the DKAN ddev addon adds an extra config file called
+    # config.dkan.yml. This prevents us from just calling
+    # `ddev config --php-version` to change PHP version. In the future,
+    # change this around so we can just tell ddev to change the PHP version.
     parameters:
-      upgrade:
-        description: "If true, will install the latest stable version and test upgrade"
-        default: false
-        type: boolean
+      php_version:
+        description: 'PHP major.minor for DDev to use.'
+        default: '7.4'
+        type: string
+      addon_branch:
+        description: 'Repo branch name for the dkan-ddev-addon you want to test against.'
+        default: 'main'
+        type: string
     steps:
       - run:
           name: Set up composer config
@@ -33,7 +45,13 @@ commands:
             which ddev
             ddev --version
             ddev config --project-name test-$CIRCLE_WORKFLOW_JOB_ID --project-type drupal9 --docroot docroot --create-docroot
-            ddev get https://github.com/GetDKAN/dkan-ddev-addon/archive/refs/heads/main.tar.gz
+            ddev get https://github.com/GetDKAN/dkan-ddev-addon/archive/refs/heads/<< parameters.addon_branch >>.tar.gz
+            # Modify config to use our PHP version.
+            sed -i 's/^php_version.*$/php_version: "<< parameters.php_version >>"/' .ddev/config.dkan.yaml
+            # Clear out omit_containers.
+            sed -i '/omit_containers/d' .ddev/config.dkan.yaml
+            # Don't load the DBA container.
+            echo "omit_containers: [dba]" >> .ddev/config.dkan.yaml
             ddev restart
             ddev status
             ddev dkan-init --force
@@ -41,14 +59,36 @@ commands:
             chmod +x ./cc-test-reporter
             mkdir -p test_results
 
+  prepare_drupal_core:
+    # TODO: Hopefully getdkan/recommended-project will have tagged releases at some point.
+    parameters:
+      drupal_core_constraint:
+        description: 'Drupal core version to use as a Composer constraint.'
+        default: '~9.5.0'
+        type: string
+    steps:
+      - run:
+          name: Use Drupal core version
+          command: |
+            ddev composer show "drupal/core*" --no-ansi
+            # Remove drupal/core-recommended and swap for drupal/core so that
+            # we can install core 9.5 using PHP 8.1. (Guzzle would block otherwise.)
+            ddev composer remove drupal/core-recommended --no-update
+            ddev composer require drupal/core-composer-scaffold:<< parameters.drupal_core_constraint >> --no-update
+            ddev composer require drupal/core-project-message:<< parameters.drupal_core_constraint >> --no-update
+            ddev composer require drupal/core:<< parameters.drupal_core_constraint >> --no-update
+            ddev composer require --dev drupal/core-dev:<< parameters.drupal_core_constraint >> --no-update
+            ddev composer update --no-audit
+            ddev composer show "drupal/core*" --no-ansi
+
   prepare_site:
     parameters:
       upgrade:
-        description: "If true, will install the latest stable version and test upgrade"
+        description: 'If true, will install the latest stable version and test upgrade'
         default: false
         type: boolean
       needs_cypress:
-        description: "If true, will add Cypress to the DDev environment"
+        description: 'If true, will add Cypress to the DDev environment'
         default: false
         type: boolean
     steps:
@@ -95,34 +135,35 @@ commands:
 
 jobs:
   phpunit:
-    machine:
-      image: ubuntu-2004:current
+    executor: docker/machine
     parameters:
-      upgrade:
-        description: "If true, will install the latest stable version and test upgrade"
+      php_version:
+        description: 'PHP major.minor for DDev to use.'
+        default: '8.0'
+        type: string
+      drupal_core_constraint:
+        description: 'Drupal core version to use as a Composer constraint.'
+        default: '~9.5.0'
+        type: string
+      report_coverage:
+        description: 'Generate coverage report and send it to CodeClimate'
         default: false
         type: boolean
-      needs_cypress:
-        description: "If true, will add Cypress to the DDev environment"
+      upgrade:
+        description: 'If true, will install the latest stable DKAN version and test upgrade'
         default: false
         type: boolean
     steps:
+      - docker/install-docker-tools
       - prepare_build:
-          upgrade: << parameters.upgrade >>
+          php_version: << parameters.php_version >>
+      - prepare_drupal_core:
+          drupal_core_constraint: << parameters.drupal_core_constraint >>
       - prepare_site:
           upgrade: << parameters.upgrade >>
-          needs_cypress: << parameters.needs_cypress >>
+          needs_cypress: false
       - when:
-          condition: << parameters.upgrade >>
-          steps:
-            - run:
-                name: Run PHPUnit tests
-                command: |
-                  ddev dkan-test-phpunit \
-                    --log-junit ../test_results/phpunit_results.xml
-      - unless:
-          # Only collect coverage info for non-upgrade PHPUnit tests.
-          condition: << parameters.upgrade >>
+          condition: << parameters.report_coverage >>
           steps:
             - run:
                 name: Run PHPUnit tests with coverage report
@@ -133,7 +174,8 @@ jobs:
                   $CIRCLE_WORKING_DIRECTORY/cc-test-reporter before-build
                   ddev dkan-test-phpunit \
                     --coverage-clover /var/www/html/docroot/modules/contrib/dkan/clover.xml \
-                    --log-junit ../test_results/phpunit_results.xml
+                    --coverage-html /var/www/html/docroot/modules/contrib/dkan/coverage-html \
+                    --log-junit /var/www/html/docroot/modules/contrib/dkan/junit/junit.xml
                   TEST_RESULT=$?
                   if [ -f docroot/modules/contrib/dkan/clover.xml ]; then
                     echo "Coverage file: docroot/modules/contrib/dkan/clover.xml"
@@ -146,34 +188,53 @@ jobs:
                     --prefix /var/www/html/dkan \
                     --exit-code $TEST_RESULT
                   exit $TEST_RESULT
+            - store_artifacts:
+                path: docroot/modules/contrib/dkan/coverage-html
+      - unless:
+          condition: << parameters.report_coverage >>
+          steps:
+            - run:
+                name: Run PHPUnit tests
+                command: |
+                  ddev dkan-test-phpunit \
+                    --log-junit /var/www/html/docroot/modules/contrib/dkan/junit/junit.xml
       - store_test_results:
-          path: test_results
+          path: dkan/junit
 
   cypress:
     machine:
       image: ubuntu-2004:current
     parallelism: 4
     parameters:
+      php_version:
+        description: 'PHP major.minor for DDev to use.'
+        default: '8.0'
+        type: string
+      drupal_core_constraint:
+        description: 'Drupal core version to use as a Composer constraint.'
+        default: '~9.5.0'
+        type: string
       upgrade:
-        description: "If true, will install the latest stable version and test upgrade"
-        default: false
-        type: boolean
-      needs_cypress:
-        description: "If true, will add Cypress to the DDev environment"
+        description: 'If true, will install the latest stable DKAN version and test upgrade'
         default: false
         type: boolean
     steps:
+      - docker/install-docker-tools
       - prepare_build:
-          upgrade: << parameters.upgrade >>
+          php_version: << parameters.php_version >>
+      - prepare_drupal_core:
+          drupal_core_constraint: << parameters.drupal_core_constraint >>
       - prepare_site:
           upgrade: << parameters.upgrade >>
-          needs_cypress: << parameters.needs_cypress >>
+          needs_cypress: true
       - run:
           name: Run Cypress tests
           command: |
-            mkdir dkan/cypress/tmp && mkdir dkan/cypress/results
+            mkdir dkan/cypress/tmp
+            mkdir dkan/cypress/results
             mv $(circleci tests glob dkan/cypress/integration/*.spec.js | circleci tests split --split-by=timings) dkan/cypress/tmp || true
-            rm dkan/cypress/integration/* && mv dkan/cypress/tmp/* dkan/cypress/integration
+            rm dkan/cypress/integration/*
+            mv dkan/cypress/tmp/* dkan/cypress/integration
             ddev dkan-module-test-cypress \
               --headless \
               --reporter junit \
@@ -188,18 +249,41 @@ jobs:
 workflows:
   version: 2
   install_and_test:
+    # We use matrix for these parameters so that we can tell them apart in report screens.
     jobs:
-      - phpunit:
-          name: install_test_phpunit
       - cypress:
           name: install_test_cypress
-          needs_cypress: true
+      - phpunit:
+          matrix:
+            parameters:
+              drupal_core_constraint: [ '~9.4.0' ]
+              php_version: [ '7.4' ]
+      - phpunit:
+          matrix:
+            parameters:
+              drupal_core_constraint: [ '~9.5.0' ]
+              php_version: [ '7.4', '8.1' ]
+      - phpunit:
+          name: 'Install target (Drupal 9.5, PHP 8.0)'
+          report_coverage: true
+          matrix:
+            parameters:
+              drupal_core_constraint: [ '~9.5.0' ]
+              php_version: [ '8.0' ]
+  #      - phpunit:
+  #          matrix:
+  #            parameters:
+  #              drupal_core_constraint: [ '~10.0.0' ]
+  #              php_version: [ '8.1' ]
   upgrade_and_test:
     jobs:
-      - phpunit:
-          name: upgrade_test_phpunit
-          upgrade: true
       - cypress:
           name: upgrade_test_cypress
           upgrade: true
-          needs_cypress: true
+      - phpunit:
+          name: 'Upgrade target (Drupal 9.5, PHP 8.0)'
+          upgrade: true
+          matrix:
+            parameters:
+              drupal_core_constraint: [ '~9.5.0' ]
+              php_version: [ '8.0' ]
diff --git a/composer.json b/composer.json
index 1eddb981b2..a836efa269 100644
--- a/composer.json
+++ b/composer.json
@@ -6,7 +6,7 @@
     "description": "DKAN Open Data Catalog",
     "require": {
         "drupal/admin_toolbar": "^3",
-        "drupal/config_update": "^1.6",
+        "drupal/config_update": "^2@dev",
         "drupal/moderated_content_bulk_publish": "~2.0.20",
         "drupal/search_api": "^1.15",
         "drupal/select_or_other": "dev-4.x#ae0585ff8c",
@@ -14,16 +14,16 @@
         "drupal/views_bulk_operations": "^4.0",
         "ext-json": "*",
         "ezyang/htmlpurifier" : "^4.11",
-        "fmizzell/maquina": "^1.1.0",
+        "fmizzell/maquina": "^1.1.1",
         "getdkan/contracts": "^1.0.0",
-        "getdkan/csv-parser": "^1.2.3",
-        "getdkan/file-fetcher" : "4.2.0",
+        "getdkan/csv-parser": "^1.3.0",
+        "getdkan/file-fetcher" : "^4.2.1",
         "getdkan/harvest": "^1.0.0",
         "getdkan/json-schema-provider": "^0.1.2",
         "getdkan/locker": "^1.1.0",
         "getdkan/procrastinator": "^4.0.0",
         "getdkan/rooted-json-data": "^0.1",
-        "getdkan/sql-parser": "^2.0.0",
+        "getdkan/sql-parser": "^2.1.0",
         "guzzlehttp/guzzle" : "^6.5.8 || ^7.4.5",
         "ilbee/csv-response": "^1.1.1",
         "m1x0n/opis-json-schema-error-presenter": "^0.5.3",
diff --git a/modules/metastore/tests/src/Unit/Reference/ReferencerTest.php b/modules/metastore/tests/src/Unit/Reference/ReferencerTest.php
index 64a7f8034c..868b2ef9ab 100644
--- a/modules/metastore/tests/src/Unit/Reference/ReferencerTest.php
+++ b/modules/metastore/tests/src/Unit/Reference/ReferencerTest.php
@@ -23,7 +23,7 @@
 use Drupal\node\Entity\Node;
 use Drupal\node\NodeStorage;
 
-use GuzzleHttp\Exception\RequestException;
+use GuzzleHttp\Exception\ConnectException;
 use MockChain\Chain;
 use MockChain\Options;
 use PHPUnit\Framework\TestCase;
@@ -38,23 +38,23 @@ class ReferencerTest extends TestCase {
    *
    * @var string
    */
-  const FILE_PATH = 'tmp/mycsv.csv';
+  public const FILE_PATH = 'tmp/mycsv.csv';
 
   /**
    * HTTP host protocol and domain for testing download URL.
    *
    * @var string
    */
-  const HOST = 'http://example.com';
+  public const HOST = 'http://example.com';
 
-  const MIME_TYPE = 'text/csv';
+  public const MIME_TYPE = 'text/csv';
 
   /**
    * List referenceable dataset properties.
    *
    * @var string[]
    */
-  const REFERENCEABLE_PROPERTY_LIST = [
+  public const REFERENCEABLE_PROPERTY_LIST = [
     'keyword' => 0,
     'distribution' => 'distribution',
     'title' => 0,
@@ -453,7 +453,7 @@ public function getMimeType() { return ReferencerTest::MIME_TYPE; }
     $this->assertEquals(self::MIME_TYPE, $container_chain->getStoredInput('resource')[0]->getMimeType(), 'Unable to fetch MIME type for remote file');
     // Test Mime Type detection on a invalid remote file path.
     $data = $this->getData('http://invalid');
-    $this->expectException(RequestException::class);
+    $this->expectException(ConnectException::class);
     $referencer->reference($data);
   }
 

From 2d6e7ea99eac83d2ddc2f29d7a7e94de341691e5 Mon Sep 17 00:00:00 2001
From: Paul Mitchum <paul@mile23.com>
Date: Thu, 30 Mar 2023 14:20:16 -0700
Subject: [PATCH 3/9] minor improvement

---
 modules/common/src/DataResource.php | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/modules/common/src/DataResource.php b/modules/common/src/DataResource.php
index 8ef5d24ad0..6615fb5088 100644
--- a/modules/common/src/DataResource.php
+++ b/modules/common/src/DataResource.php
@@ -101,12 +101,10 @@ public function __construct($file_path, $mimeType, $perspective = self::DEFAULT_
    *
    * @param object $record
    *   Data resource record from the database. Must contain these properties:
-   *   - filePath
-   *   - mimeType
-   *   - perspective
-   *   - version
+   *   'filePath', 'mimeType', 'perspective', 'version'.
    *
    * @return \Drupal\common\DataResource
+   *   DataResource object.
    */
   public static function createFromRecord(object $record): DataResource {
     $resource = new static($record->filePath, $record->mimeType, $record->perspective);
@@ -232,6 +230,8 @@ public function getPerspective() {
 
   /**
    * Getter.
+   *
+   * @return string
    */
   public function getUniqueIdentifier() {
     return self::buildUniqueIdentifier($this->identifier, $this->version, $this->perspective);

From cbd0af1c1353cc7d94b9b559aa5743c4337c42cb Mon Sep 17 00:00:00 2001
From: Paul Mitchum <paul@mile23.com>
Date: Thu, 30 Mar 2023 16:37:03 -0700
Subject: [PATCH 4/9] minor updates

---
 .../src/Unit/Plugin/QueueWorker/ImportJobTest.php  | 14 +++++++-------
 .../src/Storage/MetastoreStorageInterface.php      |  4 ++--
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php b/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php
index bbbe67497f..671b2e8ce9 100644
--- a/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php
+++ b/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php
@@ -49,7 +49,7 @@ private function getDatastore(DatastoreResource $resource) {
    */
   public function testBasics() {
     $resource = new DatastoreResource(1, __DIR__ . "/../../../../data/countries.csv", "text/csv");
-    $this->assertEquals($resource->getID(), 1);
+    $this->assertEquals(1, $resource->getID());
 
     $datastore = $this->getDatastore($resource);
 
@@ -150,7 +150,7 @@ public function testColumnNameSpaces() {
   public function testSerialization() {
     $timeLimit = 40;
     $resource = new DatastoreResource(1, __DIR__ . "/../../../../data/countries.csv", "text/csv");
-    $this->assertEquals($resource->getID(), 1);
+    $this->assertEquals(1, $resource->getID());
 
     $datastore = $this->getDatastore($resource);
     $datastore->setTimeLimit($timeLimit);
@@ -216,10 +216,10 @@ public function testMultiplePasses() {
    */
   public function testBadStorage() {
     $storageInterfaceClass = DatabaseTableInterface::class;
-    $this->expectExceptionMessage("Storage must be an instance of {$storageInterfaceClass}");
+    $this->expectExceptionMessage("Storage must be an instance of $storageInterfaceClass");
     $resource = new DatastoreResource(1, __DIR__ . "/../../../../data/countries.csZv", "text/csv");
 
-    $importer = ImportJob::get("1", new Memory(), [
+    ImportJob::get("1", new Memory(), [
       "resource" => $resource,
       "storage" => new TestMemStorageBad(),
       "parser" => Csv::getParser(),
@@ -232,7 +232,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");
-    $importer = ImportJob::get("1", new Memory(), [
+    ImportJob::get("1", new Memory(), [
       "resource" => $resource,
       "storage" => new class {
       },
@@ -240,7 +240,7 @@ public function testNonStorage() {
     ]);
   }
 
-  public function sanitizeDescriptionProvider() {
+  public function sanitizeDescriptionProvider(): array {
     return [
       'multiline' => ["Multi\nLine", 'Multi Line'],
     ];
@@ -269,7 +269,7 @@ public function testSanitizeHeader($column, $expected) {
     $this->assertEquals($expected, ImportJob::sanitizeHeader($column));
   }
 
-  public function truncateHeaderProvider() {
+  public function truncateHeaderProvider(): array {
     $max_length = 64;
     return [
       'max_length' => [
diff --git a/modules/metastore/src/Storage/MetastoreStorageInterface.php b/modules/metastore/src/Storage/MetastoreStorageInterface.php
index 9a926cdc16..2db518b9b2 100644
--- a/modules/metastore/src/Storage/MetastoreStorageInterface.php
+++ b/modules/metastore/src/Storage/MetastoreStorageInterface.php
@@ -114,7 +114,7 @@ public function remove(string $id);
   /**
    * Store.
    *
-   * @param string|HydratableInterface $data
+   * @param string $data
    *   The data to be stored.
    * @param string $id
    *   The identifier for the data. If the act of storing generates the
@@ -126,7 +126,7 @@ public function remove(string $id);
    * @throws \Exception
    *   Issues storing the data.
    */
-  public function store($data, string $id = NULL): string;
+  public function store(string $data, string $id = NULL): string;
 
   /**
    * Retrieve by hash.

From fe609dbb07d01d0e733e2bfa750de2cf56729dfa Mon Sep 17 00:00:00 2001
From: Paul Mitchum <paul@mile23.com>
Date: Thu, 30 Mar 2023 17:04:32 -0700
Subject: [PATCH 5/9] document deprecation

---
 .../tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php  | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php b/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php
index 671b2e8ce9..e5207f3dc4 100644
--- a/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php
+++ b/modules/datastore/tests/src/Unit/Plugin/QueueWorker/ImportJobTest.php
@@ -13,6 +13,9 @@
 
 /**
  * Unit tests for Importer class.
+ *
+ * @group datastore
+ * @group dkan-core
  */
 class ImportJobTest extends TestCase {
 
@@ -34,7 +37,7 @@ protected function tearDown(): void {
   /**
    *
    */
-  private function getDatastore(DatastoreResource $resource) {
+  private function getDatastore(DatastoreResource $resource): ImportJob {
     $storage = new Memory();
     $config = [
       "resource" => $resource,
@@ -145,7 +148,11 @@ public function testColumnNameSpaces() {
   }
 
   /**
+   * Test JSON/hydrate round-trip.
    *
+   * This pattern is deprecated.
+   *
+   * @group legacy
    */
   public function testSerialization() {
     $timeLimit = 40;

From 454134a64ae04cc0e11368a9d1f466b7599658c6 Mon Sep 17 00:00:00 2001
From: Paul Mitchum <paul@mile23.com>
Date: Sun, 16 Apr 2023 14:39:08 -0500
Subject: [PATCH 6/9] removed DataResource::createCommon

---
 modules/common/src/DataResource.php           | 34 +++++---------
 ...{ResourceTest.php => DataResourceTest.php} | 47 ++++++++++++++++++-
 modules/metastore/src/ResourceMapper.php      |  1 -
 phpunit.xml                                   |  9 ----
 4 files changed, 58 insertions(+), 33 deletions(-)
 rename modules/common/tests/src/Unit/{ResourceTest.php => DataResourceTest.php} (58%)

diff --git a/modules/common/src/DataResource.php b/modules/common/src/DataResource.php
index 6615fb5088..07e9a113b4 100644
--- a/modules/common/src/DataResource.php
+++ b/modules/common/src/DataResource.php
@@ -116,7 +116,7 @@ public static function createFromRecord(object $record): DataResource {
   }
 
   /**
-   * Create a new version.
+   * Clone the current resource with a new version identifier.
    *
    * Versions are, simply, a unique "string" used to represent changes in a
    * resource. For example, when new data is added to a file/resource a new
@@ -126,17 +126,18 @@ public static function createFromRecord(object $record): DataResource {
    * resources, it simply models the behavior to allow other parts of the
    * system to create new versions of resources when they deem it necessary.
    */
-  public function createNewVersion() {
+  public function createNewVersion(): DataResource {
     $newVersion = time();
     if ($newVersion == $this->version) {
       $newVersion++;
     }
-
-    return $this->createCommon('version', $newVersion);
+    $clone = clone $this;
+    $clone->version = $newVersion;
+    return $clone;
   }
 
   /**
-   * Create a new perspective.
+   * Clone the current resource with a new perspective.
    *
    * Perspectives are useful to represent clusters of connected resources.
    *
@@ -146,11 +147,11 @@ public function createNewVersion() {
    * aware of the new resource, the API endpoint, and maintain the relatioship
    * between the 2 resources.
    */
-  public function createNewPerspective($perspective, $uri) {
-    $new = $this->createCommon('perspective', $perspective);
-    $new->changeFilePath($uri);
-
-    return $new;
+  public function createNewPerspective($perspective, $uri): DataResource {
+    $clone = clone $this;
+    $clone->perspective = $perspective;
+    $clone->changeFilePath($uri);
+    return $clone;
   }
 
   /**
@@ -167,18 +168,6 @@ public function changeMimeType($newMimeType) {
     $this->mimeType = $newMimeType;
   }
 
-  /**
-   * Private.
-   */
-  private function createCommon($property, $value) {
-    $current = $this->{$property};
-    $new = $value;
-    $this->{$property} = $new;
-    $newResource = clone $this;
-    $this->{$property} = $current;
-    return $newResource;
-  }
-
   /**
    * Get object storing datastore specific information about this resource.
    *
@@ -232,6 +221,7 @@ public function getPerspective() {
    * Getter.
    *
    * @return string
+   *   The unique identifier.
    */
   public function getUniqueIdentifier() {
     return self::buildUniqueIdentifier($this->identifier, $this->version, $this->perspective);
diff --git a/modules/common/tests/src/Unit/ResourceTest.php b/modules/common/tests/src/Unit/DataResourceTest.php
similarity index 58%
rename from modules/common/tests/src/Unit/ResourceTest.php
rename to modules/common/tests/src/Unit/DataResourceTest.php
index c636027359..834b07cecf 100644
--- a/modules/common/tests/src/Unit/ResourceTest.php
+++ b/modules/common/tests/src/Unit/DataResourceTest.php
@@ -12,8 +12,10 @@
 
 /**
  * Unit tests for Drupal\common\Resource.
+ *
+ * @coversDefaultClass \Drupal\common\DataResource
  */
-class ResourceTest extends TestCase {
+class DataResourceTest extends TestCase {
 
   /**
    * Test getTableName().
@@ -82,4 +84,47 @@ public function testGetIdentifierAndVersionException() {
     $result = DataResource::getIdentifierAndVersion($id);
   }
 
+  /**
+   * @covers ::createNewPerspective
+   */
+  public function testCreateNewPerspective() {
+    $data_resource = new DataResource(
+      '/foo/bar',
+      'txt',
+      DataResource::DEFAULT_SOURCE_PERSPECTIVE
+    );
+
+    $clone_data_resource = $data_resource->createNewPerspective('local_url', 'uri://foo/bar');
+
+    // Not the same object.
+    $this->assertNotSame($data_resource, $clone_data_resource);
+    // Clone contains 'local_url' perspective.
+    $ref_perspective = new \ReflectionProperty($clone_data_resource, 'perspective');
+    $ref_perspective->setAccessible(TRUE);
+    $this->assertEquals('local_url', $ref_perspective->getValue($clone_data_resource));
+  }
+
+  /**
+   * @covers ::createNewVersion
+   */
+  public function testCreateNewVersion() {
+    $data_resource = new DataResource(
+      '/foo/bar',
+      'txt',
+      DataResource::DEFAULT_SOURCE_PERSPECTIVE
+    );
+
+    $clone_data_resource = $data_resource->createNewVersion();
+
+    // Not the same object.
+    $this->assertNotSame($data_resource, $clone_data_resource);
+    // Clone contains new version.
+    $ref_version = new \ReflectionProperty(DataResource::class, 'version');
+    $ref_version->setAccessible(TRUE);
+    $this->assertNotEquals(
+      $ref_version->getValue($data_resource),
+      $ref_version->getValue($clone_data_resource)
+    );
+  }
+
 }
diff --git a/modules/metastore/src/ResourceMapper.php b/modules/metastore/src/ResourceMapper.php
index 265e58e1d5..8f0955e509 100644
--- a/modules/metastore/src/ResourceMapper.php
+++ b/modules/metastore/src/ResourceMapper.php
@@ -8,7 +8,6 @@
 use Drupal\common\EventDispatcherTrait;
 use Drupal\datastore\Service\ResourceLocalizer;
 use Drupal\metastore\Exception\AlreadyRegistered;
-use Drupal\metastore\NodeWrapper\Data;
 
 /**
  * Map resource URLs to local files.
diff --git a/phpunit.xml b/phpunit.xml
index 3ec3332fd3..bdeb809091 100644
--- a/phpunit.xml
+++ b/phpunit.xml
@@ -26,31 +26,22 @@
       <directory>.</directory>
     </testsuite>
     <testsuite name="DkanUnitTests">
-      <directory>tests/src/Unit</directory>
       <directory>modules/common/tests/src/Unit</directory>
-      <directory>modules/metastore/tests/src/Unit</directory>
-      <directory>modules/metastore/modules/metastore/tests/src/Unit</directory>
       <directory>modules/metastore/modules/metastore_search/tests/src/Unit</directory>
-      <directory>modules/metastore/modules/metastore_admin/tests/src/Unit</directory>
       <directory>modules/datastore/tests/src/Unit</directory>
       <directory>modules/datastore/modules/datastore_mysql_import/tests/src/Unit</directory>
       <directory>modules/frontend/tests/src/Unit</directory>
       <directory>modules/dkan_js_frontend/tests/src</directory>
       <directory>modules/harvest/tests/src/Unit</directory>
-      <directory>modules/harvest/modules/harvest_dashboard/tests/src/Unit</directory>
       <directory>modules/json_form_widget/tests/src/Unit</directory>
     </testsuite>
     <testsuite name="DkanFunctionalTests">
       <directory>modules/common/tests/src/Functional</directory>
       <directory>modules/metastore/tests/src/Functional</directory>
-      <directory>modules/metastore/modules/metastore/tests/src/Functional</directory>
       <directory>modules/metastore/modules/metastore_search/tests/src/Functional</directory>
       <directory>modules/metastore/modules/metastore_admin/tests/src/Functional</directory>
       <directory>modules/datastore/tests/src/Functional</directory>
-      <directory>modules/frontend/tests/src/Functional</directory>
       <directory>modules/dkan_js_frontend/tests/src</directory>
-      <directory>modules/harvest/tests/src/Functional</directory>
-      <directory>modules/harvest/modules/harvest_dashboard/tests/src/Functional</directory>
       <directory>tests/src/Functional</directory>
     </testsuite>
   </testsuites>

From e47e2611e75ea5147ab6c6c6d9a7a5626058faa2 Mon Sep 17 00:00:00 2001
From: Paul Mitchum <paul@mile23.com>
Date: Sun, 16 Apr 2023 14:50:14 -0500
Subject: [PATCH 7/9] better docblock

---
 modules/metastore/src/Reference/Referencer.php | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/modules/metastore/src/Reference/Referencer.php b/modules/metastore/src/Reference/Referencer.php
index 9cbad78d40..b3d41105bf 100644
--- a/modules/metastore/src/Reference/Referencer.php
+++ b/modules/metastore/src/Reference/Referencer.php
@@ -212,9 +212,19 @@ private function registerWithResourceMapper(string $downloadUrl, string $mimeTyp
   }
 
   /**
-   * Private.
+   * Get download URL for existing resource.
+   *
+   * @param array $info
+   *   Info.
+   * @param \Drupal\common\DataResource $stored
+   *   Stored data resource object.
+   * @param string $mimeType
+   *   MIME type.
+   *
+   * @return string
+   *   The download URL.
    */
-  private function handleExistingResource($info, $stored, $mimeType) {
+  private function handleExistingResource(array $info, DataResource $stored, string $mimeType): string {
     if ($info[0]->perspective == DataResource::DEFAULT_SOURCE_PERSPECTIVE &&
       (ResourceMapper::newRevision() == 1 || $stored->getMimeType() != $mimeType)) {
       $new = $stored->createNewVersion();

From 2678e44f03345cdedd23f174eded43cf97d72ac2 Mon Sep 17 00:00:00 2001
From: Paul Mitchum <paul@mile23.com>
Date: Mon, 17 Apr 2023 11:43:50 -0500
Subject: [PATCH 8/9] remove todo

---
 modules/metastore/tests/src/Unit/Reference/ReferencerTest.php | 1 -
 1 file changed, 1 deletion(-)

diff --git a/modules/metastore/tests/src/Unit/Reference/ReferencerTest.php b/modules/metastore/tests/src/Unit/Reference/ReferencerTest.php
index 868b2ef9ab..ac1fa8313a 100644
--- a/modules/metastore/tests/src/Unit/Reference/ReferencerTest.php
+++ b/modules/metastore/tests/src/Unit/Reference/ReferencerTest.php
@@ -275,7 +275,6 @@ public function testChangeMediaType() {
     }';
     $data = json_decode($json);
     $referencer->reference($data);
-    // @todo: Where does this stored input come from?
     $storedResource = DataResource::createFromRecord(json_decode($container_chain->getStoredInput('resource')[0]));
     // A new resource should have been stored, with the mimetype set to text/csv
     $this->assertEquals('text/csv', $storedResource->getMimeType());

From b7545a9ed43b7eceab9fa095da44492bce4882fa Mon Sep 17 00:00:00 2001
From: Paul Mitchum <paul@mile23.com>
Date: Wed, 19 Apr 2023 09:00:24 -0500
Subject: [PATCH 9/9] dont use reflection

---
 modules/common/tests/src/Unit/DataResourceTest.php | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/modules/common/tests/src/Unit/DataResourceTest.php b/modules/common/tests/src/Unit/DataResourceTest.php
index 834b07cecf..3d20998e2a 100644
--- a/modules/common/tests/src/Unit/DataResourceTest.php
+++ b/modules/common/tests/src/Unit/DataResourceTest.php
@@ -99,9 +99,8 @@ public function testCreateNewPerspective() {
     // Not the same object.
     $this->assertNotSame($data_resource, $clone_data_resource);
     // Clone contains 'local_url' perspective.
-    $ref_perspective = new \ReflectionProperty($clone_data_resource, 'perspective');
-    $ref_perspective->setAccessible(TRUE);
-    $this->assertEquals('local_url', $ref_perspective->getValue($clone_data_resource));
+    $this->assertEquals('local_url', $clone_data_resource->getPerspective());
+    $this->assertEquals('uri://foo/bar', $clone_data_resource->getFilePath());
   }
 
   /**
@@ -119,11 +118,9 @@ public function testCreateNewVersion() {
     // Not the same object.
     $this->assertNotSame($data_resource, $clone_data_resource);
     // Clone contains new version.
-    $ref_version = new \ReflectionProperty(DataResource::class, 'version');
-    $ref_version->setAccessible(TRUE);
     $this->assertNotEquals(
-      $ref_version->getValue($data_resource),
-      $ref_version->getValue($clone_data_resource)
+      $data_resource->getVersion(),
+      $clone_data_resource->getVersion()
     );
   }