diff --git a/modules/common/src/Storage/AbstractJobStoreFactory.php b/modules/common/src/Storage/AbstractJobStoreFactory.php new file mode 100644 index 0000000000..5a3f043a6c --- /dev/null +++ b/modules/common/src/Storage/AbstractJobStoreFactory.php @@ -0,0 +1,69 @@ +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); + } + +} diff --git a/modules/common/src/Storage/DeprecatedJobStoreFactoryTrait.php b/modules/common/src/Storage/DeprecatedJobStoreFactoryTrait.php new file mode 100644 index 0000000000..a9227a4f9a --- /dev/null +++ b/modules/common/src/Storage/DeprecatedJobStoreFactoryTrait.php @@ -0,0 +1,81 @@ +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; + } + +} diff --git a/modules/common/src/Storage/JobStoreFactory.php b/modules/common/src/Storage/JobStoreFactory.php index 5fab8fae71..ac1cf2b65f 100644 --- a/modules/common/src/Storage/JobStoreFactory.php +++ b/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; - } - } diff --git a/modules/common/tests/src/Kernel/Storage/AbstractJobStoreFactoryTest.php b/modules/common/tests/src/Kernel/Storage/AbstractJobStoreFactoryTest.php new file mode 100644 index 0000000000..21e8058811 --- /dev/null +++ b/modules/common/tests/src/Kernel/Storage/AbstractJobStoreFactoryTest.php @@ -0,0 +1,160 @@ +container->get('database'); + // Make a concrete AbstractJobStoreFactory object. + /** @var \DkanTestConcreteAbstractJobStoreFactory $job_store_factory */ + $job_store_factory = new \DkanTestConcreteAbstractJobStoreFactory($db); + // Get a Job object by specifying an instance name. + $job_store = $job_store_factory->getInstance(\DkanTestConcreteJobSubclass::class); + + $this->assertInstanceOf(JobStore::class, $job_store); + + // First, get the table name without deprecation by calculating a hash. + $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->assertEquals('jobstore_580088250_dkantestconcretejobsubclass', $table_name); + + // 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_factory, 'getDeprecatedTableName'); + $ref_get_deprecated_table_name->setAccessible(TRUE); + $deprecated_table_name = $ref_get_deprecated_table_name->invokeArgs( + $job_store_factory, + [\DkanTestConcreteJobSubclass::class] + ); + $this->assertEquals('jobstore_dkantestconcretejobsubclass', $deprecated_table_name); + $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) + ); + + // Create a new JobStore object. The factory should see that the + // deprecated table already exists and try to use it as the table name. + $job_store = $job_store_factory->getInstance(\DkanTestConcreteJobSubclass::class); + $this->assertEquals( + $deprecated_table_name, + $ref_get_table_name->invoke($job_store) + ); + + // Remove the table and create yet another job store object. This + // one should have the non-deprecated table name. + $job_store->destruct(); + $this->assertFalse( + $db->schema()->tableExists($deprecated_table_name) + ); + $this->assertFalse( + $db->schema()->tableExists($table_name) + ); + $job_store = $job_store_factory->getInstance(\DkanTestConcreteJobSubclass::class); + $this->assertEquals($table_name, $ref_get_table_name->invoke($job_store)); + $this->assertEquals(0, $job_store->count()); + $this->assertTrue( + $db->schema()->tableExists($table_name) + ); + + // Use an identifier string that contains backslash, but is not an + // existing class. + $job_store->destruct(); + $job_store = $job_store_factory->getInstance('test\\thisshouldneverbeaclassname'); + $this->assertEquals( + 'jobstore_2087357147_thisshouldneverbeaclassname', + $table_name = $ref_get_table_name->invoke($job_store) + ); + $this->assertEquals(0, $job_store->count()); + $this->assertTrue( + $db->schema()->tableExists($table_name) + ); + + // Use an identifier string that doesn't look like a class name. + $identifier = 'testthisshouldneverbeaclassname'; + $job_store->destruct(); + $job_store = $job_store_factory->getInstance($identifier); + $this->assertEquals( + 'jobstore_' . $identifier, + $table_name = $ref_get_table_name->invoke($job_store) + ); + $this->assertEquals(0, $job_store->count()); + $this->assertTrue( + $db->schema()->tableExists($table_name) + ); + + // And finally, the right way. We create an instance without specifying + // an identifier, so that the factory uses its default. + $job_store->destruct(); + $job_store = $job_store_factory->getInstance(); + $this->assertEquals( + 'concrete_job_store', + $table_name = $ref_get_table_name->invoke($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 Drupal\common\Storage\AbstractJobStoreFactory; + use Procrastinator\Job\Job; + + class DkanTestConcreteAbstractJobStoreFactory extends AbstractJobStoreFactory { + + protected string $tableName = 'concrete_job_store'; + + } + + class DkanTestConcreteJobSubclass extends Job { + + protected function runIt() { + } + + } +} diff --git a/modules/common/tests/src/Kernel/Storage/JobStoreFactoryTest.php b/modules/common/tests/src/Kernel/Storage/JobStoreFactoryTest.php index 4f0778f499..3a7bb1b620 100644 --- a/modules/common/tests/src/Kernel/Storage/JobStoreFactoryTest.php +++ b/modules/common/tests/src/Kernel/Storage/JobStoreFactoryTest.php @@ -10,12 +10,14 @@ use Drupal\KernelTests\KernelTestBase; /** - * @covers \Drupal\common\Storage\JobStore - * @coversDefaultClass \Drupal\common\Storage\JobStore + * @covers \Drupal\common\Storage\JobStoreFactory + * @coversDefaultClass \Drupal\common\Storage\JobStoreFactory * * @group dkan * @group common * @group kernel + * + * @see @see \Drupal\Tests\common\Kernel\Storage\AbstractJobStoreFactoryTest */ class JobStoreFactoryTest extends KernelTestBase { diff --git a/modules/datastore/datastore.services.yml b/modules/datastore/datastore.services.yml index 89677b6c0b..30e6262eed 100644 --- a/modules/datastore/datastore.services.yml +++ b/modules/datastore/datastore.services.yml @@ -6,6 +6,7 @@ services: - '@dkan.datastore.service.factory.import' - '@queue' - '@dkan.common.job_store' + - '@dkan.datastore.import_job_store_factory' - '@dkan.datastore.service.resource_processor.dictionary_enforcer' dkan.datastore.service.post_import: @@ -54,7 +55,7 @@ services: dkan.datastore.service.factory.import: class: \Drupal\datastore\Service\Factory\ImportServiceFactory arguments: - - '@dkan.common.job_store' + - '@dkan.datastore.import_job_store_factory' - '@dkan.datastore.database_table_factory' dkan.datastore.logger_channel: @@ -94,6 +95,7 @@ services: - '@dkan.datastore.service' - '@dkan.datastore.service.resource_purger' - '@dkan.common.job_store' + - '@dkan.datastore.import_job_store_factory' tags: - { name: event_subscriber } @@ -111,6 +113,11 @@ services: - '@dkan.common.job_store' - '@dkan.datastore.import_info' + dkan.datastore.import_job_store_factory: + class: \Drupal\datastore\Storage\ImportJobStoreFactory + arguments: + - '@database' + pdlt.converter.strptime_to_mysql: class: \PDLT\Converter arguments: diff --git a/modules/datastore/modules/datastore_mysql_import/datastore_mysql_import.services.yml b/modules/datastore/modules/datastore_mysql_import/datastore_mysql_import.services.yml index 2e96abfe54..bf6d0fcc73 100644 --- a/modules/datastore/modules/datastore_mysql_import/datastore_mysql_import.services.yml +++ b/modules/datastore/modules/datastore_mysql_import/datastore_mysql_import.services.yml @@ -2,7 +2,7 @@ services: dkan.datastore.service.factory.import: class: \Drupal\datastore_mysql_import\Factory\MysqlImportFactory arguments: - - '@dkan.common.job_store' + - '@dkan.datastore.import_job_store_factory' - '@dkan.datastore_mysql_import.database_table_factory' dkan.datastore_mysql_import.database_table_factory: diff --git a/modules/datastore/modules/datastore_mysql_import/src/Factory/MysqlImportFactory.php b/modules/datastore/modules/datastore_mysql_import/src/Factory/MysqlImportFactory.php index 3a0a8280d9..cfe5767d8a 100644 --- a/modules/datastore/modules/datastore_mysql_import/src/Factory/MysqlImportFactory.php +++ b/modules/datastore/modules/datastore_mysql_import/src/Factory/MysqlImportFactory.php @@ -2,9 +2,9 @@ namespace Drupal\datastore_mysql_import\Factory; -use Drupal\common\Storage\JobStoreFactory; use Drupal\datastore\Service\Factory\ImportFactoryInterface; use Drupal\datastore\Service\ImportService; +use Drupal\datastore\Storage\ImportJobStoreFactory; use Drupal\datastore_mysql_import\Service\MysqlImport; use Drupal\datastore_mysql_import\Storage\MySqlDatabaseTableFactory; @@ -16,9 +16,9 @@ class MysqlImportFactory implements ImportFactoryInterface { /** * The JobStore Factory service. * - * @var \Drupal\common\Storage\JobStoreFactory + * @var \Drupal\datastore\Storage\ImportJobStoreFactory */ - private $jobStoreFactory; + private ImportJobStoreFactory $importJobStoreFactory; /** * Database table factory service. @@ -30,8 +30,8 @@ class MysqlImportFactory implements ImportFactoryInterface { /** * Constructor. */ - public function __construct(JobStoreFactory $jobStoreFactory, MySqlDatabaseTableFactory $databaseTableFactory) { - $this->jobStoreFactory = $jobStoreFactory; + public function __construct(ImportJobStoreFactory $importJobStoreFactory, MySqlDatabaseTableFactory $databaseTableFactory) { + $this->importJobStoreFactory = $importJobStoreFactory; $this->databaseTableFactory = $databaseTableFactory; } @@ -46,7 +46,7 @@ public function getInstance(string $identifier, array $config = []) { throw new \Exception("config['resource'] is required"); } - $importer = new ImportService($resource, $this->jobStoreFactory, $this->databaseTableFactory); + $importer = new ImportService($resource, $this->importJobStoreFactory, $this->databaseTableFactory); $importer->setImporterClass(MysqlImport::class); return $importer; } diff --git a/modules/datastore/src/DatastoreService.php b/modules/datastore/src/DatastoreService.php index 625fa9e533..77c07cf00e 100644 --- a/modules/datastore/src/DatastoreService.php +++ b/modules/datastore/src/DatastoreService.php @@ -5,11 +5,11 @@ use Drupal\common\DataResource; use Drupal\common\Storage\JobStoreFactory; use Drupal\datastore\Service\ImportService; +use Drupal\datastore\Storage\ImportJobStoreFactory; use Procrastinator\Result; use Symfony\Component\DependencyInjection\ContainerInterface; use Drupal\Core\DependencyInjection\ContainerInjectionInterface; use Drupal\Core\Queue\QueueFactory; -use Drupal\datastore\Plugin\QueueWorker\ImportJob; use Drupal\datastore\Service\Factory\ImportFactoryInterface; use Drupal\datastore\Service\ResourceLocalizer; use Drupal\datastore\Service\ResourceProcessor\DictionaryEnforcer; @@ -55,6 +55,13 @@ class DatastoreService implements ContainerInjectionInterface { */ private $dictionaryEnforcer; + /** + * Import job store factory. + * + * @var \Drupal\datastore\Storage\ImportJobStoreFactory + */ + private ImportJobStoreFactory $importJobStoreFactory; + /** * {@inheritdoc} */ @@ -64,6 +71,7 @@ public static function create(ContainerInterface $container) { $container->get('dkan.datastore.service.factory.import'), $container->get('queue'), $container->get('dkan.common.job_store'), + $container->get('dkan.datastore.import_job_store_factory'), $container->get('dkan.datastore.service.resource_processor.dictionary_enforcer') ); } @@ -79,6 +87,8 @@ public static function create(ContainerInterface $container) { * Queue factory service. * @param \Drupal\common\Storage\JobStoreFactory $jobStoreFactory * Jobstore factory service. + * @param \Drupal\datastore\Storage\ImportJobStoreFactory $importJobStoreFactory + * Import jobstore factory service. * @param \Drupal\datastore\Service\ResourceProcessor\DictionaryEnforcer $dictionaryEnforcer * Dictionary Enforcer object. */ @@ -87,12 +97,14 @@ public function __construct( ImportFactoryInterface $importServiceFactory, QueueFactory $queue, JobStoreFactory $jobStoreFactory, + ImportJobStoreFactory $importJobStoreFactory, DictionaryEnforcer $dictionaryEnforcer ) { $this->queue = $queue; $this->resourceLocalizer = $resourceLocalizer; $this->importServiceFactory = $importServiceFactory; $this->jobStoreFactory = $jobStoreFactory; + $this->importJobStoreFactory = $importJobStoreFactory; $this->dictionaryEnforcer = $dictionaryEnforcer; } @@ -223,8 +235,7 @@ public function drop(string $identifier, ?string $version = NULL, bool $local_re if ($storage) { $storage->destruct(); - $this->jobStoreFactory - ->getInstance(ImportJob::class) + $this->importJobStoreFactory->getInstance() ->remove(md5($resource->getUniqueIdentifier())); } diff --git a/modules/datastore/src/EventSubscriber/DatastoreSubscriber.php b/modules/datastore/src/EventSubscriber/DatastoreSubscriber.php index 1067b1bc86..45e687e1c8 100644 --- a/modules/datastore/src/EventSubscriber/DatastoreSubscriber.php +++ b/modules/datastore/src/EventSubscriber/DatastoreSubscriber.php @@ -2,20 +2,18 @@ namespace Drupal\datastore\EventSubscriber; -use Drupal\Core\Config\ConfigFactoryInterface; -use Drupal\Core\Logger\LoggerChannelFactory; - use Drupal\common\Events\Event; use Drupal\common\DataResource; use Drupal\common\Storage\JobStoreFactory; +use Drupal\Core\Config\ConfigFactoryInterface; +use Drupal\Core\Logger\LoggerChannelFactory; use Drupal\datastore\DatastoreService; use Drupal\datastore\Service\ResourceLocalizer; use Drupal\datastore\Service\ResourcePurger; +use Drupal\datastore\Storage\ImportJobStoreFactory; use Drupal\metastore\LifeCycle\LifeCycle; use Drupal\metastore\MetastoreItemInterface; use Drupal\metastore\ResourceMapper; - -use Drupal\datastore\Plugin\QueueWorker\ImportJob; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -38,6 +36,13 @@ class DatastoreSubscriber implements EventSubscriberInterface { */ protected $loggerFactory; + /** + * Import job store factory. + * + * @var \Drupal\datastore\Storage\ImportJobStoreFactory + */ + private ImportJobStoreFactory $importJobStoreFactory; + /** * Inherited. * @@ -49,7 +54,8 @@ public static function create(ContainerInterface $container) { $container->get('logger.factory'), $container->get('dkan.datastore.service'), $container->get('dkan.datastore.service.resource_purger'), - $container->get('dkan.common.job_store') + $container->get('dkan.common.job_store'), + $container->get('dkan.datastore.import_job_store_factory') ); } @@ -66,13 +72,23 @@ public static function create(ContainerInterface $container) { * The dkan.datastore.service.resource_purger service. * @param \Drupal\common\Storage\JobStoreFactory $jobStoreFactory * The dkan.common.job_store service. + * @param \Drupal\datastore\Storage\ImportJobStoreFactory $importJobStoreFactory + * The dkan.datastore.import_job_store_factory service. */ - public function __construct(ConfigFactoryInterface $config_factory, LoggerChannelFactory $logger_factory, DatastoreService $service, ResourcePurger $resourcePurger, JobStoreFactory $jobStoreFactory) { + public function __construct( + ConfigFactoryInterface $config_factory, + LoggerChannelFactory $logger_factory, + DatastoreService $service, + ResourcePurger $resourcePurger, + JobStoreFactory $jobStoreFactory, + ImportJobStoreFactory $importJobStoreFactory + ) { $this->configFactory = $config_factory; $this->loggerFactory = $logger_factory; $this->service = $service; $this->resourcePurger = $resourcePurger; $this->jobStoreFactory = $jobStoreFactory; + $this->importJobStoreFactory = $importJobStoreFactory; } /** @@ -152,7 +168,7 @@ public function drop(Event $event) { ]); } try { - $this->jobStoreFactory->getInstance(ImportJob::class)->remove($id); + $this->importJobStoreFactory->getInstance()->remove($id); } catch (\Exception $e) { $this->loggerFactory->get('datastore')->error('Failed to remove importer job. @message', diff --git a/modules/datastore/src/Service/Factory/Import.php b/modules/datastore/src/Service/Factory/Import.php index 0c025fbbc9..4f5ddc317e 100644 --- a/modules/datastore/src/Service/Factory/Import.php +++ b/modules/datastore/src/Service/Factory/Import.php @@ -3,7 +3,7 @@ namespace Drupal\datastore\Service\Factory; use Drupal\datastore\Storage\DatabaseTableFactory; -use Drupal\common\Storage\JobStoreFactory; +use Drupal\datastore\Storage\ImportJobStoreFactory; /** * Create an importer object for a given resource. @@ -15,9 +15,14 @@ class Import extends ImportServiceFactory { /** * Constructor. + * + * @param \Drupal\datastore\Storage\ImportJobStoreFactory $importJobStoreFactory + * Import job store factory service. + * @param \Drupal\datastore\Storage\DatabaseTableFactory $databaseTableFactory + * Database table factory. */ - public function __construct(JobStoreFactory $jobStoreFactory, DatabaseTableFactory $databaseTableFactory) { - parent::__construct($jobStoreFactory, $databaseTableFactory); + public function __construct(ImportJobStoreFactory $importJobStoreFactory, DatabaseTableFactory $databaseTableFactory) { + parent::__construct($importJobStoreFactory, $databaseTableFactory); @trigger_error(__NAMESPACE__ . '\Import is deprecated. Use \Drupal\datastore\Service\Factory\ImportServiceFactory instead.', E_USER_DEPRECATED); } diff --git a/modules/datastore/src/Service/Factory/ImportServiceFactory.php b/modules/datastore/src/Service/Factory/ImportServiceFactory.php index 0a54f9ff91..fe69c3fa4d 100644 --- a/modules/datastore/src/Service/Factory/ImportServiceFactory.php +++ b/modules/datastore/src/Service/Factory/ImportServiceFactory.php @@ -4,7 +4,7 @@ use Drupal\datastore\Storage\DatabaseTableFactory; use Drupal\datastore\Service\ImportService; -use Drupal\common\Storage\JobStoreFactory; +use Drupal\datastore\Storage\ImportJobStoreFactory; /** * Create an importer object for a given resource. @@ -14,9 +14,9 @@ class ImportServiceFactory implements ImportFactoryInterface { /** * Job store factory. * - * @var \Drupal\common\Storage\JobStoreFactory + * @var \Drupal\datastore\Storage\ImportJobStoreFactory */ - private $jobStoreFactory; + private ImportJobStoreFactory $importJobStoreFactory; /** * Database table factory. @@ -35,8 +35,8 @@ class ImportServiceFactory implements ImportFactoryInterface { /** * Constructor. */ - public function __construct(JobStoreFactory $jobStoreFactory, DatabaseTableFactory $databaseTableFactory) { - $this->jobStoreFactory = $jobStoreFactory; + public function __construct(ImportJobStoreFactory $importJobStoreFactory, DatabaseTableFactory $databaseTableFactory) { + $this->importJobStoreFactory = $importJobStoreFactory; $this->databaseTableFactory = $databaseTableFactory; } @@ -52,7 +52,7 @@ public function getInstance(string $identifier, array $config = []) { } $resource = $config['resource']; - return new ImportService($resource, $this->jobStoreFactory, $this->databaseTableFactory); + return new ImportService($resource, $this->importJobStoreFactory, $this->databaseTableFactory); } } diff --git a/modules/datastore/src/Service/Import.php b/modules/datastore/src/Service/Import.php index 09a5dd7ba9..f4c34b1f1a 100644 --- a/modules/datastore/src/Service/Import.php +++ b/modules/datastore/src/Service/Import.php @@ -3,8 +3,8 @@ namespace Drupal\datastore\Service; use Drupal\common\DataResource; -use Drupal\common\Storage\JobStoreFactory; use Drupal\datastore\Storage\DatabaseTableFactory; +use Drupal\datastore\Storage\ImportJobStoreFactory; /** * Datastore import service. @@ -19,13 +19,13 @@ class Import extends ImportService { * * @param \Drupal\common\DataResource $resource * DKAN Resource. - * @param \Drupal\common\Storage\JobStoreFactory $jobStoreFactory - * Jobstore factory. + * @param \Drupal\datastore\Storage\ImportJobStoreFactory $importJobStoreFactory + * Import jobstore factory. * @param \Drupal\datastore\Storage\DatabaseTableFactory $databaseTableFactory * Database Table factory. */ - public function __construct(DataResource $resource, JobStoreFactory $jobStoreFactory, DatabaseTableFactory $databaseTableFactory) { - parent::__construct($resource, $jobStoreFactory, $databaseTableFactory); + public function __construct(DataResource $resource, ImportJobStoreFactory $importJobStoreFactory, DatabaseTableFactory $databaseTableFactory) { + parent::__construct($resource, $importJobStoreFactory, $databaseTableFactory); @trigger_error(__NAMESPACE__ . '\Import is deprecated. Use \Drupal\datastore\Service\ImportService instead.', E_USER_DEPRECATED); } diff --git a/modules/datastore/src/Service/ImportService.php b/modules/datastore/src/Service/ImportService.php index bf4cbf9ec2..8f6778737d 100644 --- a/modules/datastore/src/Service/ImportService.php +++ b/modules/datastore/src/Service/ImportService.php @@ -7,9 +7,9 @@ use Drupal\common\EventDispatcherTrait; use Drupal\common\LoggerTrait; use Drupal\common\DataResource; -use Drupal\common\Storage\JobStoreFactory; use Drupal\datastore\Storage\DatabaseTable; use Drupal\datastore\Storage\DatabaseTableFactory; +use Drupal\datastore\Storage\ImportJobStoreFactory; use Procrastinator\Result; /** @@ -54,11 +54,9 @@ class ImportService { /** * The jobstore factory service. * - * @var \Drupal\common\Storage\JobStoreFactory - * - * @todo Can we remove this? + * @var \Drupal\datastore\Storage\ImportJobStoreFactory */ - private JobStoreFactory $jobStoreFactory; + private ImportJobStoreFactory $importJobStoreFactory; /** * Database table factory service. @@ -83,14 +81,14 @@ class ImportService { * * @param \Drupal\common\DataResource $resource * DKAN Resource. - * @param \Drupal\common\Storage\JobStoreFactory $jobStoreFactory - * Jobstore factory. + * @param \Drupal\datastore\Storage\ImportJobStoreFactory $importJobStoreFactory + * Import jobstore factory. * @param \Drupal\datastore\Storage\DatabaseTableFactory $databaseTableFactory * Database Table factory. */ - public function __construct(DataResource $resource, JobStoreFactory $jobStoreFactory, DatabaseTableFactory $databaseTableFactory) { + public function __construct(DataResource $resource, ImportJobStoreFactory $importJobStoreFactory, DatabaseTableFactory $databaseTableFactory) { $this->resource = $resource; - $this->jobStoreFactory = $jobStoreFactory; + $this->importJobStoreFactory = $importJobStoreFactory; $this->databaseTableFactory = $databaseTableFactory; } @@ -158,7 +156,7 @@ public function getImporter(): ImportJob { $this->importJob = call_user_func([$this->importerClass, 'get'], $datastore_resource->getId(), - $this->jobStoreFactory->getInstance(ImportJob::class), + $this->importJobStoreFactory->getInstance(), [ "storage" => $this->getStorage(), "parser" => $this->getNonRecordingParser($delimiter), diff --git a/modules/datastore/src/Storage/ImportJobStoreFactory.php b/modules/datastore/src/Storage/ImportJobStoreFactory.php new file mode 100644 index 0000000000..318708f23a --- /dev/null +++ b/modules/datastore/src/Storage/ImportJobStoreFactory.php @@ -0,0 +1,19 @@ +container->get('dkan.datastore.service.factory.import'), $this->container->get('queue'), $this->container->get('dkan.common.job_store'), + $this->container->get('dkan.datastore.import_job_store_factory'), $this->container->get('dkan.datastore.service.resource_processor.dictionary_enforcer'), ]) ->onlyMethods(['import']) @@ -90,6 +91,7 @@ public function testRequeue() { $this->container->get('dkan.datastore.service.factory.import'), $this->container->get('queue'), $this->container->get('dkan.common.job_store'), + $this->container->get('dkan.datastore.import_job_store_factory'), $this->container->get('dkan.datastore.service.resource_processor.dictionary_enforcer'), ]) ->onlyMethods(['import']) @@ -228,6 +230,7 @@ public function testAlreadyImported() { $this->container->get('dkan.datastore.service.factory.import'), $this->container->get('queue'), $this->container->get('dkan.common.job_store'), + $this->container->get('dkan.datastore.import_job_store_factory'), $this->container->get('dkan.datastore.service.resource_processor.dictionary_enforcer'), ]) ->onlyMethods(['getStorage']) diff --git a/modules/datastore/tests/src/Kernel/Service/ImportServiceTest.php b/modules/datastore/tests/src/Kernel/Service/ImportServiceTest.php index 9d0311303c..d588705026 100644 --- a/modules/datastore/tests/src/Kernel/Service/ImportServiceTest.php +++ b/modules/datastore/tests/src/Kernel/Service/ImportServiceTest.php @@ -50,7 +50,7 @@ public function testImport() { ->onlyMethods(['getImporter']) ->setConstructorArgs([ new DataResource('abc.txt', 'text/csv'), - $this->container->get('dkan.common.job_store'), + $this->container->get('dkan.datastore.import_job_store_factory'), $this->container->get('dkan.datastore.database_table_factory'), ]) ->getMock(); diff --git a/modules/datastore/tests/src/Unit/DatastoreServiceTest.php b/modules/datastore/tests/src/Unit/DatastoreServiceTest.php index 9b4bba21d2..6e3dab7f4e 100644 --- a/modules/datastore/tests/src/Unit/DatastoreServiceTest.php +++ b/modules/datastore/tests/src/Unit/DatastoreServiceTest.php @@ -5,6 +5,7 @@ use Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher; use Drupal\Core\Queue\QueueFactory; use Drupal\datastore\Plugin\QueueWorker\ImportJob; +use Drupal\datastore\Storage\ImportJobStoreFactory; use Drupal\Tests\common\Traits\ServiceCheckTrait; use Drupal\common\DataResource; use Drupal\common\Storage\JobStore; @@ -63,6 +64,7 @@ public function testDrop() { ->add(DatabaseTable::class, 'destruct') ->add(ResourceLocalizer::class, 'remove') ->add(JobStoreFactory::class, 'getInstance', JobStore::class) + ->add(ImportJobStoreFactory::class, 'getInstance', JobStore::class) ->add(JobStore::class, 'remove', TRUE); $service = DatastoreService::create($mockChain->getMock()); @@ -96,6 +98,7 @@ private function getCommonChain() { ->add('dkan.datastore.service.factory.import', ImportServiceFactory::class) ->add('queue', QueueFactory::class) ->add('dkan.common.job_store', JobStoreFactory::class) + ->add('dkan.datastore.import_job_store_factory', ImportJobStoreFactory::class) ->add('dkan.datastore.import_info_list', ImportInfoList::class) ->add('dkan.datastore.service.resource_processor.dictionary_enforcer', DictionaryEnforcer::class) ->index(0); diff --git a/modules/datastore/tests/src/Unit/EventSubscriber/DatastoreSubscriberTest.php b/modules/datastore/tests/src/Unit/EventSubscriber/DatastoreSubscriberTest.php index e141aefd04..8375b1956d 100644 --- a/modules/datastore/tests/src/Unit/EventSubscriber/DatastoreSubscriberTest.php +++ b/modules/datastore/tests/src/Unit/EventSubscriber/DatastoreSubscriberTest.php @@ -14,6 +14,7 @@ use Drupal\datastore\Service\ResourcePurger; use Drupal\datastore\Storage\DatabaseTable; use Drupal\common\Events\Event; +use Drupal\datastore\Storage\ImportJobStoreFactory; use MockChain\Chain; use MockChain\Options; use PHPUnit\Framework\TestCase; @@ -103,6 +104,7 @@ public function testDrop() { ->add('dkan.datastore.service', DatastoreService::class) ->add('dkan.datastore.service.resource_purger', ResourcePurger::class) ->add('dkan.common.job_store', JobStoreFactory::class) + ->add('dkan.datastore.import_job_store_factory', ImportJobStoreFactory::class) ->add("database", Connection::class) ->index(0); @@ -113,6 +115,7 @@ public function testDrop() { ->add(ImportServiceFactory::class, 'getInstance', ImportService::class) ->add(ImportService::class, 'remove') ->add(JobStoreFactory::class, 'getInstance', JobStore::class) + ->add(ImportJobStoreFactory::class, 'getInstance', JobStore::class) ->add(JobStore::class, 'remove') ->add(LoggerChannelFactory::class, 'get', LoggerChannelInterface::class) ->add(LoggerChannelInterface::class, 'error', NULL, 'errors') @@ -138,6 +141,7 @@ public function testDatastoreDropException() { ->add('dkan.datastore.service', DatastoreService::class) ->add('dkan.datastore.service.resource_purger', ResourcePurger::class) ->add('dkan.common.job_store', JobStoreFactory::class) + ->add('dkan.datastore.import_job_store_factory', ImportJobStoreFactory::class) ->add("database", Connection::class) ->index(0); @@ -147,6 +151,7 @@ public function testDatastoreDropException() { ->add(ImportServiceFactory::class, 'getInstance', ImportService::class) ->add(ImportService::class, 'remove') ->add(JobStoreFactory::class, 'getInstance', JobStore::class) + ->add(ImportJobStoreFactory::class, 'getInstance', JobStore::class) ->add(JobStore::class, 'remove') ->add(LoggerChannelFactory::class, 'get', LoggerChannelInterface::class) ->add(LoggerChannelInterface::class, 'error', NULL, 'errors') @@ -171,6 +176,7 @@ public function testJobStoreRemoveException() { ->add('dkan.datastore.service', DatastoreService::class) ->add('dkan.datastore.service.resource_purger', ResourcePurger::class) ->add('dkan.common.job_store', JobStoreFactory::class) + ->add('dkan.datastore.import_job_store_factory', ImportJobStoreFactory::class) ->add("database", Connection::class) ->index(0); @@ -180,6 +186,7 @@ public function testJobStoreRemoveException() { ->add(ImportServiceFactory::class, 'getInstance', ImportService::class) ->add(ImportService::class, 'remove') ->add(JobStoreFactory::class, 'getInstance', JobStore::class) + ->add(ImportJobStoreFactory::class, 'getInstance', JobStore::class) ->add(JobStore::class, 'remove', new \Exception('error')) ->add(LoggerChannelFactory::class, 'get', LoggerChannelInterface::class) ->add(LoggerChannelInterface::class, 'error', NULL, 'errors') @@ -200,6 +207,7 @@ private function getContainerChain() { ->add('dkan.datastore.service', DatastoreService::class) ->add('dkan.datastore.service.resource_purger', ResourcePurger::class) ->add('dkan.common.job_store', JobStoreFactory::class) + ->add('dkan.datastore.import_job_store_factory', ImportJobStoreFactory::class) ->index(0); return (new Chain($this)) diff --git a/modules/datastore/tests/src/Unit/Service/DatastoreQueryTest.php b/modules/datastore/tests/src/Unit/Service/DatastoreQueryTest.php index a2d93f076e..cc49b89f0e 100644 --- a/modules/datastore/tests/src/Unit/Service/DatastoreQueryTest.php +++ b/modules/datastore/tests/src/Unit/Service/DatastoreQueryTest.php @@ -6,6 +6,7 @@ use Drupal\Core\DependencyInjection\Container; use Drupal\common\Storage\JobStoreFactory; use Drupal\Core\Queue\QueueFactory; +use Drupal\datastore\Storage\ImportJobStoreFactory; use Drupal\Tests\datastore\Traits\TestHelperTrait; use MockChain\Chain; use MockChain\Options; @@ -194,6 +195,7 @@ public function getCommonMockChain() { ->add('queue', QueueFactory::class) ->add('request_stack', RequestStack::class) ->add('dkan.common.job_store', JobStoreFactory::class) + ->add('dkan.datastore.import_job_store_factory', ImportJobStoreFactory::class) ->add('dkan.metastore.storage', DataFactory::class) ->add('dkan.datastore.import_info_list', ImportInfoList::class) ->add('dkan.datastore.service.resource_processor.dictionary_enforcer', DictionaryEnforcer::class) diff --git a/modules/datastore/tests/src/Unit/Service/Factory/ImportServiceFactoryTest.php b/modules/datastore/tests/src/Unit/Service/Factory/ImportServiceFactoryTest.php index f898d72e55..c63c924545 100644 --- a/modules/datastore/tests/src/Unit/Service/Factory/ImportServiceFactoryTest.php +++ b/modules/datastore/tests/src/Unit/Service/Factory/ImportServiceFactoryTest.php @@ -2,9 +2,9 @@ namespace Drupal\Tests\datastore\Unit\Service\Factory; -use Drupal\common\Storage\JobStoreFactory; use Drupal\datastore\Storage\DatabaseTableFactory; use Drupal\datastore\Service\Factory\ImportServiceFactory; +use Drupal\datastore\Storage\ImportJobStoreFactory; use PHPUnit\Framework\TestCase; /** @@ -21,7 +21,7 @@ class ImportServiceFactoryTest extends TestCase { */ public function testGetInstanceException() { $factory = new ImportServiceFactory( - $this->getMockBuilder(JobStoreFactory::class) + $this->getMockBuilder(ImportJobStoreFactory::class) ->disableOriginalConstructor() ->getMock(), $this->getMockBuilder(DatabaseTableFactory::class)