Consolidate draft workflow tests, other test fixes (#4760)

Author: dafeder

tests/src/Functional/DatasetBTBTest.php

@@ -66,8 +66,11 @@ public function testResourcePurgeDraft() {
 
     // Post, update and publish a dataset with multiple, changing resources.
     $this->storeDatasetRunQueues($id_1, '1.1', ['1.csv', '2.csv'], 'post');
+    // These updates reuse 1.csv, so guard against version timestamp collisions.
+    $this->avoidResourceVersionCollision();
     $this->storeDatasetRunQueues($id_1, '1.2', ['3.csv', '1.csv'], 'put');
     $this->getMetastore()->publish('dataset', $id_1);
+    $this->avoidResourceVersionCollision();
     $this->storeDatasetRunQueues($id_1, '1.3', ['1.csv', '5.csv'], 'put');
 
     /** @var \Drupal\dkan_common\DatasetInfo $datasetInfo */
@@ -198,75 +201,51 @@ public function testHarvestOrphan() {
   }
 
   /**
-   * Test draft moderation workflow with distribution url update and default source resource perspective.
+   * Provide draft workflow/perspective combinations.
    */
-  public function testDraftWorkflowDistributionUrlSourcePerspective() {
-    // Set resource perspective to source.
-    $this->config('dkan_metastore.settings')
-      ->set('resource_perspective_display', DataResource::DEFAULT_SOURCE_PERSPECTIVE)
-      ->save();
-
-    $this->runDraftWorkflowUpdateDistributionUrl();
-  }
-
-  /**
-   * Test draft moderation workflow with distribution url update and local_url source resource perspective.
-   */
-  public function testDraftWorkflowDistributionUrlLocalPerspective() {
-    // Set resource perspective to source.
-    $this->config('dkan_metastore.settings')
-      ->set('resource_perspective_display', ResourceLocalizer::LOCAL_URL_PERSPECTIVE)
-      ->save();
-
-    $this->runDraftWorkflowUpdateDistributionUrl();
-  }
-
-  /**
-   * Test draft moderation workflow with modified trigger and default source resource perspective.
-   */
-  public function testDraftWorkflowModifiedTriggerSourcePerspective() {
-    // Set resource perspective to source.
-    $this->config('dkan_metastore.settings')
-      ->set('resource_perspective_display', DataResource::DEFAULT_SOURCE_PERSPECTIVE)
-      ->save();
-
-    $this->runDraftWorkflowModifiedTrigger();
-  }
-
-  /**
-   * Test draft moderation workflow with modified trigger and local_url resource perspective.
-   */
-  public function testDraftWorkflowModifiedTriggerLocalPerspective() {
-    // Set resource perspective to local_url.
-    $this->config('dkan_metastore.settings')
-      ->set('resource_perspective_display', ResourceLocalizer::LOCAL_URL_PERSPECTIVE)
-      ->save();
-
-    $this->runDraftWorkflowModifiedTrigger();
-  }
-
-  /**
-   * Test draft moderation workflow with distribution title update and source resource perspective.
-   */
-  public function testDraftWorkflowUpdateDistributionTitleSourcePerspective() {
-    // Set resource perspective to local_url.
-    $this->config('dkan_metastore.settings')
-      ->set('resource_perspective_display', DataResource::DEFAULT_SOURCE_PERSPECTIVE)
-      ->save();
-
-    $this->runDraftWorkflowUpdateDistributionTitle();
+  public static function draftWorkflowPerspectiveProvider(): array {
+    $cases = [
+      'url_source' => [
+        DataResource::DEFAULT_SOURCE_PERSPECTIVE,
+        'runDraftWorkflowUpdateDistributionUrl',
+      ],
+      'url_local_url' => [
+        ResourceLocalizer::LOCAL_URL_PERSPECTIVE,
+        'runDraftWorkflowUpdateDistributionUrl',
+      ],
+      'modified_source' => [
+        DataResource::DEFAULT_SOURCE_PERSPECTIVE,
+        'runDraftWorkflowModifiedTrigger',
+      ],
+      'modified_local_url' => [
+        ResourceLocalizer::LOCAL_URL_PERSPECTIVE,
+        'runDraftWorkflowModifiedTrigger',
+      ],
+      'title_source' => [
+        DataResource::DEFAULT_SOURCE_PERSPECTIVE,
+        'runDraftWorkflowUpdateDistributionTitle',
+      ],
+      'title_local_url' => [
+        ResourceLocalizer::LOCAL_URL_PERSPECTIVE,
+        'runDraftWorkflowUpdateDistributionTitle',
+      ],
+    ];
+    return $cases;
   }
 
   /**
-   * Test draft moderation workflow with distribution title update and local_url resource perspective.
+   * Test draft workflows across resource perspectives and dist modes.
+   *
+   * @dataProvider draftWorkflowPerspectiveProvider
    */
-  public function testDraftWorkflowUpdateDistributionTitleLocalPerspective() {
-    // Set resource perspective to local_url.
+  public function testDraftWorkflowScenarios(
+    string $resource_perspective_display,
+    string $workflow_method,
+  ): void {
     $this->config('dkan_metastore.settings')
-      ->set('resource_perspective_display', ResourceLocalizer::LOCAL_URL_PERSPECTIVE)
+      ->set('resource_perspective_display', $resource_perspective_display)
       ->save();
-
-    $this->runDraftWorkflowUpdateDistributionTitle();
+    $this->{$workflow_method}();
   }
 
   /**
@@ -305,7 +284,9 @@ public function testOrphanDraftDistributionCleanup() {
     // Confirm distribution local directory exists.
     $this->assertDirectoryExists('public://resources/' . $resourceDirectory);
 
-    // Update the modified date for the dataset.
+    // Update the modified date for the dataset. Guard against reusing the same
+    // resource version timestamp as the original import.
+    $this->avoidResourceVersionCollision();
     $this->getMetastore()->patch('dataset', $id_1, json_encode(['modified' => '06-05-2222']));
 
     // Simulate datastore_import and cleanup queues post update.
@@ -314,6 +295,7 @@ public function testOrphanDraftDistributionCleanup() {
       'datastore_import',
       'orphan_reference_processor',
       'orphan_resource_remover',
+      'resource_purger',
     ]);
 
     // Confirm original distribution table removed.
@@ -424,7 +406,10 @@ public function testSanitizeDatasetProperties() {
   }
 
   /**
+   * Post a basic dataset and retrieve it.
    *
+   * @return object
+   *   The retrieved dataset.
    */
   private function datasetPostAndRetrieve(): object {
     $datasetRootedJsonData = $this->getData(123, 'Test #1', ['district_centerpoints_small.csv']);
@@ -451,7 +436,7 @@ private function datasetPostAndRetrieve(): object {
   }
 
   /**
-   *
+   * Import dataset into datastore perform a basic query.
    */
   private function datastoreImportAndQuery() {
     $dataset = $this->datasetPostAndRetrieve();
@@ -491,7 +476,13 @@ private function getResourceFromDataset(object $dataset) {
   }
 
   /**
+   * Get the download URL for a resource file.
    *
+   * @param string $filename
+   *   The filename of the resource.
+   *
+   * @return string
+   *   The download URL for the resource file from S3 bucket.
    */
   private function getDownloadUrl(string $filename) {
     return 'file://' . __DIR__ . '/../../files/' . $filename;
@@ -588,7 +579,10 @@ private function storeDatasetRunQueues(string $identifier, string $title, array
   }
 
   /**
+   * Count the number of datastore tables.
    *
+   * @return int
+   *   The number of datastore tables.
    */
   private function countTables() {
     /** @var \Drupal\Core\Database\Connection $db */
@@ -599,7 +593,10 @@ private function countTables() {
   }
 
   /**
+   * Return normalized CSV filenames found under public://resources.
    *
+   * @return array
+   *   Sorted array of CSV filenames (without paths or prefixes).
    */
   private function checkFiles() {
     /** @var \Drupal\Core\File\FileSystemInterface $fileSystem */
@@ -620,7 +617,10 @@ private function checkFiles() {
   }
 
   /**
+   * Query a resource using the SQL endpoint service.
    *
+   * @param string $queryString
+   *   The SQL-esque query string to run.
    */
   private function queryResource(string $queryString) {
     /** @var \Drupal\dkan_datastore\SqlEndpoint\DatastoreSqlEndpointService $sqlEndpoint */
@@ -630,7 +630,17 @@ private function queryResource(string $queryString) {
   }
 
   /**
+   * Handle HTTP verbs for dataset operations.
+   *
+   * @param string $method
+   *   The HTTP method to use ('post' or 'put').
+   * @param \RootedData\RootedJsonData $json
+   *   The JSON data for the dataset.
+   * @param object $dataset
+   *   The dataset object.
    *
+   * @return string
+   *   The dataset identifier.
    */
   private function httpVerbHandler(string $method, RootedJsonData $json, $dataset) {
 
@@ -648,21 +658,30 @@ private function httpVerbHandler(string $method, RootedJsonData $json, $dataset)
   }
 
   /**
+   * Get the harvester service from the container.
    *
+   * @return \Drupal\dkan_harvest\HarvestService
+   *   The harvester service.
    */
   private function getHarvester() : HarvestService {
     return $this->container->get('dkan.harvest.service');
   }
 
   /**
+   * Get the node storage handler from the container.
    *
+   * @return \Drupal\node\NodeStorage
+   *   The node storage handler.
    */
   private function getNodeStorage(): NodeStorage {
     return $this->container->get('entity_type.manager')->getStorage('node');
   }
 
   /**
+   * Get the metastore service from the container.
    *
+   * @return \Drupal\dkan_metastore\MetastoreService
+   *   The metastore service.
    */
   private function getMetastore(): MetastoreService {
     return $this->container->get('dkan.metastore.service');
@@ -701,11 +720,14 @@ private function createInitialDraftDatasetAndPublish(string $identifier): void {
   }
 
   /**
-   * Confirm a new datastore import took place after an update to an existing dataset (draft workflow).
+   * Confirm draft workflow led to a new datastore import and orphan cleanup.
+   *
+   * @param string $identifier
+   *   Dataset identifier.
    */
   private function confirmNewDatastoreImportDraftWorkflow(string $identifier): void {
-    // Simulate all possible queues post update.
-    // Should include datastore_import, orphan_reference_processor and resource_purger.
+    // Simulate all possible queues post update. Should include datastore
+    // import, orphan_reference_processor and resource_purger.
     $this->runQueues([
       'localize_import',
       'datastore_import',
@@ -722,7 +744,7 @@ private function confirmNewDatastoreImportDraftWorkflow(string $identifier): voi
     $distributionTablePublished = $metadata['published_revision']['distributions'][0]['table_name'] ?? '';
     $distributionUuidOld = $metadata['published_revision']['distributions'][0]['distribution_uuid'] ?? '';
 
-    // Make sure there are both latest and published versions with different tables.
+    // Make sure there are latest and published versions with different tables.
     $this->assertNotEmpty($distributionTablePublished, 'Draft revision exists.');
     $this->assertNotEquals($distributionTableLatest, $distributionTablePublished, 'Separate distribution tables exist for latest and published revisions.');
 
@@ -793,6 +815,7 @@ private function runDraftWorkflowModifiedTrigger(): void {
     $this->getMetastore()->patch('dataset', $id_1, json_encode(['modified' => '06-05-2222']));
 
     // Run queues; check that datastore import and orphan cleanup worked as expected.
+    $this->avoidResourceVersionCollision();
     $this->confirmNewDatastoreImportDraftWorkflow($id_1);
   }
 
@@ -806,14 +829,14 @@ private function runDraftWorkflowUpdateDistributionTitle(): void {
     // Create initial draft dataset and then publish it.
     $this->createInitialDraftDatasetAndPublish($id_1);
 
-    // Use same values for distribution as original getData() with updated title.
+    // Use same values for distribution as original getData() with new filepath.
     $distribution = new \stdClass();
     $distribution->title = 'Updated Distribution #0 for ' . $id_1;
     $distribution->downloadURL = $this->getDownloadUrl('1.csv');
     $distribution->format = 'csv';
     $distribution->mediaType = 'text/csv';
 
-    // Run distribution title update with cron run between update and publish events.
+    // Create a new draft with the new distribution title.
     $this->runDistributionTitleUpdate($id_1, $distribution);
 
     // Run distribution title update with cron run only after publish.
@@ -825,6 +848,7 @@ private function runDraftWorkflowUpdateDistributionTitle(): void {
    * Separate distribution title update to allow for multiple runs.
    */
   private function runDistributionTitleUpdate(string $identifier, \stdClass $distribution, bool $skip_cron = FALSE) {
+    $this->avoidResourceVersionCollision();
     // Create a new draft with the new distribution title.
     $this->getMetastore()->patch('dataset', $identifier, json_encode(
       ['distribution' => [$distribution]]
@@ -915,7 +939,7 @@ private function runDraftWorkflowUpdateDistributionUrl(): void {
     // Create initial draft dataset and then publish it.
     $this->createInitialDraftDatasetAndPublish($id_1);
 
-    // Use same values for distribution as original getData() with new file path.
+    // Use same values for distribution as original getData() with new filepath.
     $distribution = new \stdClass();
     $distribution->title = 'Distribution #0 for ' . $id_1;
     $distribution->downloadURL = $this->getDownloadUrl('2.csv');
@@ -927,8 +951,17 @@ private function runDraftWorkflowUpdateDistributionUrl(): void {
       ['distribution' => [$distribution]]
     ));
 
-    // Run queues; check that datastore import and orphan cleanup worked as expected.
+    // Run queues; check datastore import and orphan cleanup worked as expected.
     $this->confirmNewDatastoreImportDraftWorkflow($id_1);
   }
 
+  /**
+   * Sleep briefly to avoid resource version collisions in draft workflows.
+   *
+   * @todo Remove once resource versions are guaranteed unique.
+   */
+  private function avoidResourceVersionCollision(): void {
+    sleep(1);
+  }
+
 }