15750 Jobstore allow existing sites to use deprecated table name (#3987)

modules/common/src/Storage/JobStore.php

@@ -15,21 +15,30 @@ class JobStore extends AbstractDatabaseTable {
    *
    * @var string
    */
-  private $jobClass;
+  private string $jobClass;
 
   /**
-   * Store the name of the table so we do not have to recompute.
+   * Store the name of the table so that we do not have to recompute.
    *
    * @var string
    */
-  private $tableName;
+  private string $tableName;
 
   /**
    * Constructor.
+   *
+   * @param string $jobClass
+   *   Class name of the job object which is using this storage table. Must be
+   *   a subclass of \Procrastinator\Job\Job.
+   * @param \Drupal\Core\Database\Connection $connection
+   *   Database connection.
+   *
+   * @throws \Exception
+   *   Thrown if the job class is not valid.
    */
   public function __construct(string $jobClass, Connection $connection) {
     if (!$this->validateJobClass($jobClass)) {
-      throw new \Exception("Invalid jobType provided: $jobClass");
+      throw new \Exception('Invalid jobType provided: ' . $jobClass);
     }
     $this->jobClass = $jobClass;
     $this->setOurSchema();
@@ -48,21 +57,46 @@ public function retrieve(string $id) {
   }
 
   /**
-   * Protected.
+   * Get the table name, preferring the deprecated one if it exists.
+   *
+   * Since we have two table names (one deprecated), we should try to find out
+   * if the deprecated one exists. If it does, we use its name. Otherwise, we
+   * use the new table name.
+   *
+   * @todo Phase out the use of the deprecated table name.
    */
   protected function getTableName() {
     if (empty($this->tableName)) {
-      // Avoid table-name-too-long errors by hashing the FQN of the class.
-      $exploded_class = explode("\\", $this->jobClass);
-      $this->tableName = strtolower(implode('_', [
-        'jobstore',
-        crc32($this->jobClass),
-        array_pop($exploded_class),
-      ]));
+      if ($this->tableExist($table = $this->getDeprecatedTableName())) {
+        $this->tableName = $table;
+      }
+      else {
+        // Avoid table-name-too-long errors by hashing the FQN of the class.
+        $exploded_class = explode('\\', $this->jobClass);
+        $this->tableName = strtolower(implode('_', [
+          'jobstore',
+          crc32($this->jobClass),
+          array_pop($exploded_class),
+        ]));
+      }
     }
     return $this->tableName;
   }
 
+  /**
+   * Get the deprecated table name.
+   *
+   * @return string
+   *   Deprecated table name.
+   */
+  protected function getDeprecatedTableName(): string {
+    $safeClassName = strtolower(preg_replace(
+      '/\\\\/', '_',
+      $this->jobClass
+    ));
+    return 'jobstore_' . $safeClassName;
+  }
+
   /**
    * Private.
    */
@@ -112,4 +146,14 @@ public function primaryKey() {
     return 'ref_uuid';
   }
 
+  /**
+   * Drop the table if it exists.
+   *
+   * Will also drop the deprecated table if it exists.
+   */
+  public function destruct() {
+    parent::destruct();
+    $this->connection->schema()->dropTable($this->getDeprecatedTableName());
+  }
+
 }

modules/common/tests/src/Kernel/Storage/JobStoreTest.php

@@ -0,0 +1,105 @@
+<?php
+
+/**
+ * Manage namespaces so they don't get too long for deprecated table names.
+ */
+
+namespace Drupal\Tests\common\Kernel\Storage {
+
+  use Drupal\common\Storage\JobStore;
+  use Drupal\KernelTests\KernelTestBase;
+
+  /**
+   * @covers \Drupal\common\Storage\JobStore
+   * @coversDefaultClass \Drupal\common\Storage\JobStore
+   *
+   * @group dkan
+   * @group common
+   * @group kernel
+   */
+  class JobStoreTest extends KernelTestBase {
+
+    protected static $modules = [
+      'common',
+    ];
+
+    public function testDeprecatedTable() {
+      $db = $this->container->get('database');
+      // Make a JobStore object.
+      /** @var \Drupal\common\Storage\JobStoreFactory $job_store_factory */
+      $job_store_factory = $this->container->get('dkan.common.job_store');
+      $job_store = $job_store_factory->getInstance(\DkanTestJobSubclass::class);
+
+      $this->assertInstanceOf(JobStore::class, $job_store);
+
+      // First, get the table name without deprecation.
+      $ref_get_table_name = new \ReflectionMethod($job_store, 'getTableName');
+      $ref_get_table_name->setAccessible(TRUE);
+      $table_name = $ref_get_table_name->invoke($job_store);
+
+      // This table does not exist.
+      $this->assertFalse(
+        $db->schema()->tableExists($table_name)
+      );
+
+      // Use the deprecated table name.
+      $ref_get_deprecated_table_name = new \ReflectionMethod($job_store, 'getDeprecatedTableName');
+      $ref_get_deprecated_table_name->setAccessible(TRUE);
+      $deprecated_table_name = $ref_get_deprecated_table_name->invoke($job_store);
+      $ref_table_name = new \ReflectionProperty($job_store, 'tableName');
+      $ref_table_name->setAccessible(TRUE);
+      $ref_table_name->setValue($job_store, $deprecated_table_name);
+
+      // Write the table. Count() will create the table before counting rows.
+      $this->assertEquals(0, $job_store->count());
+      // Assert that the deprecated table exists.
+      $this->assertTrue(
+        $db->schema()->tableExists($deprecated_table_name)
+      );
+      // Assert that the non-deprecated table does not exist.
+      $this->assertFalse(
+        $db->schema()->tableExists($table_name)
+      );
+
+      // Unset the table name, which should cause JobStore to decide to use the
+      // deprecated table name on its own, since we already have a table for it.
+      $ref_table_name->setValue($job_store, '');
+      $this->assertEquals(
+        $deprecated_table_name,
+        $ref_get_table_name->invoke($job_store)
+      );
+
+      // Finally, remove the table, unset the job's table name, and it should
+      // create a new table with the non-deprecated name.
+      $job_store->destruct();
+      $this->assertFalse(
+        $db->schema()->tableExists($deprecated_table_name)
+      );
+      $this->assertFalse(
+        $db->schema()->tableExists($table_name)
+      );
+      $ref_table_name->setValue($job_store, '');
+      $this->assertEquals(0, $job_store->count());
+      $this->assertTrue(
+        $db->schema()->tableExists($table_name)
+      );
+    }
+
+  }
+}
+
+/**
+ * We must manage namespaces so that we don't end up with a too-long table name.
+ */
+
+namespace {
+
+  use Procrastinator\Job\Job;
+
+  class DkanTestJobSubclass extends Job {
+
+    protected function runIt() {
+    }
+
+  }
+}

modules/common/tests/src/Unit/Storage/JobStoreTest.php

@@ -17,8 +17,9 @@
 use PHPUnit\Framework\TestCase;
 
 /**
+ * @covers \Drupal\common\Storage\JobStore
  * @coversDefaultClass \Drupal\common\Storage\JobStore
- * @group datastore
+ * @group common
  */
 class JobStoreTest extends TestCase {
 

modules/datastore/src/Service/Info/ImportInfo.php

@@ -17,7 +17,7 @@ class ImportInfo {
   /**
    * A JobStore object.
    *
-   * @var \Drupal\common\Storage\JobStore
+   * @var \Drupal\common\Storage\JobStoreFactory
    */
   private $jobStoreFactory;
 

modules/datastore/src/Service/Info/ImportInfoList.php

@@ -15,7 +15,7 @@ class ImportInfoList implements ContainerInjectionInterface {
   /**
    * A JobStore object.
    *
-   * @var \Drupal\common\Storage\JobStore
+   * @var \Drupal\common\Storage\JobStoreFactory
    */
   private $jobStoreFactory;