Use literal identifier for ImportJobStoreFactory (#4083)

modules/common/src/Storage/AbstractJobStoreFactory.php

@@ -0,0 +1,69 @@
+<?php
+
+namespace Drupal\common\Storage;
+
+use Drupal\Core\Database\Connection;
+
+/**
+ * DKAN JobStore factory base class.
+ */
+abstract class AbstractJobStoreFactory implements StorageFactoryInterface {
+
+  use DeprecatedJobStoreFactoryTrait;
+
+  /**
+   * The import job store table name.
+   *
+   * Override this for your table name.
+   *
+   * @var string
+   */
+  protected string $tableName = '';
+
+  /**
+   * Drupal database connection.
+   *
+   * @var \Drupal\Core\Database\Connection
+   */
+  protected Connection $connection;
+
+  /**
+   * Constructor.
+   */
+  public function __construct(Connection $connection) {
+    $this->connection = $connection;
+  }
+
+  /**
+   * {@inheritDoc}
+   *
+   * @param string $identifier
+   *   (optional) An identifier. This is optional because unless there is an
+   *   existing table with a deprecated name, we'll use the table name from
+   *   $this->TABLE_NAME.
+   * @param array $config
+   *   (optional) Ignored, because JobStore does not use it.
+   *
+   * @return \Drupal\common\Storage\DatabaseTableInterface
+   *   Resulting JobStore object.
+   */
+  public function getInstance(string $identifier = '', array $config = []): DatabaseTableInterface {
+    $table_name = $this->tableName;
+    // For historical reasons, we keep the getInstance() method signature, but
+    // we also want to enforce our static table name.
+    if ($identifier && $identifier !== $this->tableName) {
+      // If the caller passed an unusual identifier, we should try to use the
+      // table name they desire for backwards compatibility.
+      $table_name = $this->getTableName($identifier);
+    }
+    $deprecated_table_name = $this->getDeprecatedTableName($identifier);
+    // Figure out whether we need a separate deprecated table name. This will
+    // be used in JobStore::destruct() to clean up deprecated tables if they
+    // exist.
+    if ($table_name === $deprecated_table_name) {
+      $deprecated_table_name = '';
+    }
+    return new JobStore($table_name, $this->connection, $deprecated_table_name);
+  }
+
+}

modules/common/src/Storage/DeprecatedJobStoreFactoryTrait.php

@@ -0,0 +1,81 @@
+<?php
+
+namespace Drupal\common\Storage;
+
+/**
+ * DKAN JobStore factory trait.
+ *
+ * Provides helper methods for dealing with deprecated JobStore table names.
+ */
+trait DeprecatedJobStoreFactoryTrait {
+
+  /**
+   * Get the table name, preferring the deprecated one if it exists.
+   *
+   * If the identifier contains a backslash (\), we assume it's a class name and
+   * could be either the deprecated or new hashed-style table name. We try to
+   * find out if the deprecated one exists. If it does, we use its name.
+   * Otherwise, we use the hashed-style table name.
+   *
+   * If the identifier does not contain a backslash, then we assume it is a
+   * table-name-safe string, and prepend it with 'jobstore_'.
+   *
+   * @todo Phase out the use of the deprecated and hashed-class table names in
+   *   favor of string literal identifiers.
+   */
+  protected function getTableName(string $identifier): string {
+    // Check for either \ or class_exists() because not all class names will
+    // contain a backslash.
+    if ((strpos($identifier, '\\') !== FALSE) || class_exists($identifier)) {
+      $deprecatedTableName = $this->getDeprecatedTableName($identifier);
+      if ($this->connection->schema()->tableExists($deprecatedTableName)) {
+        return $deprecatedTableName;
+      }
+      return $this->getHashedTableName($identifier);
+    }
+    return 'jobstore_' . $identifier;
+  }
+
+  /**
+   * Hash the class name identifier to generate a table name.
+   *
+   * This is the preferred strategy for new jobstore tables.
+   *
+   * @return string
+   *   The hashed table name.
+   */
+  protected function getHashedTableName(string $identifier): string {
+    // Avoid table-name-too-long errors by hashing the FQN of the identifier,
+    // assuming it's a class name.
+    $exploded_class = explode('\\', $identifier);
+    $table_name = strtolower(implode('_', [
+      'jobstore',
+      crc32($identifier),
+      array_pop($exploded_class),
+    ]));
+    return $table_name;
+  }
+
+  /**
+   * Get the deprecated table name.
+   *
+   * The deprecated table name is generated by replacing \ with _ in
+   * fully-qualified class names.
+   *
+   * This style of jobstore table name is deprecated. See release notes for
+   * DKAN 2.16.5.
+   *
+   * @return string
+   *   Deprecated table name.
+   *
+   * @see https://github.com/GetDKAN/dkan/releases/tag/2.16.5
+   */
+  protected function getDeprecatedTableName(string $identifier): string {
+    $safeClassName = strtolower(preg_replace(
+      '/\\\\/', '_',
+      $identifier
+    ));
+    return 'jobstore_' . $safeClassName;
+  }
+
+}

modules/common/src/Storage/JobStoreFactory.php

@@ -6,15 +6,24 @@
 
 /**
  * DKAN JobStore Factory.
+ *
+ * @deprecated This class still exists in code to provide backwards
+ *   compatibility.
+ *
+ * @todo Add FileFetcherJobStoreFactory as well.
+ *
+ * @see \Drupal\common\Storage\AbstractJobStoreFactory
  */
 class JobStoreFactory implements StorageFactoryInterface {
 
+  use DeprecatedJobStoreFactoryTrait;
+
   /**
    * Drupal database connection.
    *
    * @var \Drupal\Core\Database\Connection
    */
-  private Connection $connection;
+  protected Connection $connection;
 
   /**
    * Constructor.
@@ -51,73 +60,4 @@ public function getInstance(string $identifier, array $config = []): DatabaseTab
     return new JobStore($table_name, $this->connection, $deprecated_table_name);
   }
 
-  /**
-   * Get the table name, preferring the deprecated one if it exists.
-   *
-   * If the identifier contains a backslash (\), we assume it's a class name and
-   * could be either the deprecated or new hashed-style table name. We try to
-   * find out if the deprecated one exists. If it does, we use its name.
-   * Otherwise, we use the hashed-style table name.
-   *
-   * If the identifier does not contain a backslash, then we assume it is a
-   * table-name-safe string, and prepend it with 'jobstore_'.
-   *
-   * @todo Phase out the use of the deprecated and hashed-class table names in
-   *   favor of string literal identifiers.
-   */
-  protected function getTableName(string $identifier): string {
-    // Check for either \ or class_exists() because not all class names will
-    // contain a backslash.
-    if ((strpos($identifier, '\\') !== FALSE) || class_exists($identifier)) {
-      $deprecatedTableName = $this->getDeprecatedTableName($identifier);
-      if ($this->connection->schema()->tableExists($deprecatedTableName)) {
-        return $deprecatedTableName;
-      }
-      return $this->getHashedTableName($identifier);
-    }
-    return 'jobstore_' . $identifier;
-  }
-
-  /**
-   * Hash the class name identifier to generate a table name.
-   *
-   * This is the preferred strategy for new jobstore tables.
-   *
-   * @return string
-   *   The hashed table name.
-   */
-  protected function getHashedTableName(string $identifier): string {
-    // Avoid table-name-too-long errors by hashing the FQN of the identifier,
-    // assuming it's a class name.
-    $exploded_class = explode('\\', $identifier);
-    $table_name = strtolower(implode('_', [
-      'jobstore',
-      crc32($identifier),
-      array_pop($exploded_class),
-    ]));
-    return $table_name;
-  }
-
-  /**
-   * Get the deprecated table name.
-   *
-   * The deprecated table name is generated by replacing \ with _ in
-   * fully-qualified class names.
-   *
-   * This style of jobstore table name is deprecated. See release notes for
-   * DKAN 2.16.5.
-   *
-   * @return string
-   *   Deprecated table name.
-   *
-   * @see https://github.com/GetDKAN/dkan/releases/tag/2.16.5
-   */
-  protected function getDeprecatedTableName(string $identifier): string {
-    $safeClassName = strtolower(preg_replace(
-      '/\\\\/', '_',
-      $identifier
-    ));
-    return 'jobstore_' . $safeClassName;
-  }
-
 }