Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use literal identifier for ImportJobStoreFactory #4083

Merged
merged 8 commits into from
Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 69 additions & 0 deletions modules/common/src/Storage/AbstractJobStoreFactory.php
Original file line number Diff line number Diff line change
@@ -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);
}

}
81 changes: 81 additions & 0 deletions modules/common/src/Storage/DeprecatedJobStoreFactoryTrait.php
Original file line number Diff line number Diff line change
@@ -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;
}

}
80 changes: 10 additions & 70 deletions modules/common/src/Storage/JobStoreFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}

}
Loading