From c0f6e222a5698220a787a34806b0b2ab131ee27c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yann=20Eugon=C3=A9?= Date: Mon, 27 Nov 2023 11:13:03 +0100 Subject: [PATCH 1/6] Add a way for JobExecution storage to be setup before used --- .../src/DoctrineDBALJobExecutionStorage.php | 24 +++++- .../DoctrineDBALJobExecutionStorageTest.php | 26 +++---- .../src/SetupStorageCommand.php | 53 +++++++++++++ .../tests/SetupStorageCommandTest.php | 74 +++++++++++++++++++ src/batch-symfony-framework/composer.json | 1 + .../services/symfony/console/command.xml | 6 ++ src/batch-symfony-framework/tests/CliTest.php | 26 +++++++ .../SetupableJobExecutionStorageInterface.php | 16 ++++ 8 files changed, 211 insertions(+), 15 deletions(-) create mode 100644 src/batch-symfony-console/src/SetupStorageCommand.php create mode 100644 src/batch-symfony-console/tests/SetupStorageCommandTest.php create mode 100644 src/batch-symfony-framework/tests/CliTest.php create mode 100644 src/batch/src/Storage/SetupableJobExecutionStorageInterface.php diff --git a/src/batch-doctrine-dbal/src/DoctrineDBALJobExecutionStorage.php b/src/batch-doctrine-dbal/src/DoctrineDBALJobExecutionStorage.php index 3e3cbf4b..7636951a 100644 --- a/src/batch-doctrine-dbal/src/DoctrineDBALJobExecutionStorage.php +++ b/src/batch-doctrine-dbal/src/DoctrineDBALJobExecutionStorage.php @@ -20,12 +20,14 @@ use Yokai\Batch\Storage\JobExecutionStorageInterface; use Yokai\Batch\Storage\Query; use Yokai\Batch\Storage\QueryableJobExecutionStorageInterface; +use Yokai\Batch\Storage\SetupableJobExecutionStorageInterface; /** * This {@see JobExecutionStorageInterface} will store * {@see JobExecution} in an SQL database using doctrine/dbal. */ -final class DoctrineDBALJobExecutionStorage implements QueryableJobExecutionStorageInterface +final class DoctrineDBALJobExecutionStorage implements QueryableJobExecutionStorageInterface, + SetupableJobExecutionStorageInterface { private const DEFAULT_OPTIONS = [ 'table' => 'yokai_batch_job_execution', @@ -54,7 +56,7 @@ public function __construct(ConnectionRegistry $doctrine, array $options) /** * Create required table for this storage. */ - public function createSchema(): void + public function setup(): void { $assetFilter = $this->connection->getConfiguration()->getSchemaAssetsFilter(); $this->connection->getConfiguration()->setSchemaAssetsFilter(null); @@ -88,6 +90,24 @@ public function createSchema(): void $this->connection->getConfiguration()->setSchemaAssetsFilter($assetFilter); } + /** + * Create required table for this storage. + * @deprecated + */ + public function createSchema(): void + { + @\trigger_error( + \sprintf( + 'Since yokai/batch-doctrine-dbal 0.5.8: ' . + 'Method "%s()" is deprecated and will be removed in 0.6.0. Use %s::setup() instead.', + \__METHOD__, + \__CLASS__, + ), + \E_USER_DEPRECATED, + ); + $this->setup(); + } + public function store(JobExecution $execution): void { try { diff --git a/src/batch-doctrine-dbal/tests/DoctrineDBALJobExecutionStorageTest.php b/src/batch-doctrine-dbal/tests/DoctrineDBALJobExecutionStorageTest.php index ef1e989b..c91c2891 100644 --- a/src/batch-doctrine-dbal/tests/DoctrineDBALJobExecutionStorageTest.php +++ b/src/batch-doctrine-dbal/tests/DoctrineDBALJobExecutionStorageTest.php @@ -34,7 +34,7 @@ public function testCreateStandardTable(): void $schemaManager = $this->connection->getSchemaManager(); self::assertFalse($schemaManager->tablesExist(['yokai_batch_job_execution'])); - $this->createStorage()->createSchema(); + $this->createStorage()->setup(); self::assertTrue($schemaManager->tablesExist(['yokai_batch_job_execution'])); $columns = $schemaManager->listTableColumns('yokai_batch_job_execution'); @@ -61,7 +61,7 @@ public function testCreateCustomTable(): void $schemaManager = $this->connection->getSchemaManager(); self::assertFalse($schemaManager->tablesExist(['acme_job_executions'])); - $this->createStorage(['table' => 'acme_job_executions'])->createSchema(); + $this->createStorage(['table' => 'acme_job_executions'])->setup(); self::assertTrue($schemaManager->tablesExist(['acme_job_executions'])); $columns = $schemaManager->listTableColumns('acme_job_executions'); @@ -86,7 +86,7 @@ public function testCreateCustomTable(): void public function testStoreInsert(): void { $storage = $this->createStorage(); - $storage->createSchema(); + $storage->setup(); $export = JobExecution::createRoot('123', 'export', new BatchStatus(BatchStatus::RUNNING)); $export->setStartTime(new DateTimeImmutable('2021-09-23 11:05:00')); @@ -122,7 +122,7 @@ public function testStoreInsert(): void public function testStoreUpdate(): void { $storage = $this->createStorage(); - $storage->createSchema(); + $storage->setup(); $storage->store($execution = JobExecution::createRoot('123', 'export')); $execution->setStatus(BatchStatus::COMPLETED); $storage->store($execution); @@ -138,7 +138,7 @@ public function testStoreFailing(): void $this->expectException(CannotStoreJobExecutionException::class); $storage = $this->createStorage(); - /** not calling {@see DoctrineDBALJobExecutionStorage::createSchema} will cause table to not exists */ + /** not calling {@see DoctrineDBALJobExecutionStorage::setup} will cause table to not exists */ $storage->store(JobExecution::createRoot('123', 'export')); } @@ -147,7 +147,7 @@ public function testRemove(): void $this->expectException(JobExecutionNotFoundException::class); $storage = $this->createStorage(); - $storage->createSchema(); + $storage->setup(); $storage->store($execution = JobExecution::createRoot('123', 'export')); $storage->remove($execution); @@ -159,14 +159,14 @@ public function testRemoveFailing(): void $this->expectException(CannotRemoveJobExecutionException::class); $storage = $this->createStorage(); - /** not calling {@see DoctrineDBALJobExecutionStorage::createSchema} will cause table to not exists */ + /** not calling {@see DoctrineDBALJobExecutionStorage::setup} will cause table to not exists */ $storage->remove(JobExecution::createRoot('123', 'export')); } public function testRetrieve(): void { $storage = $this->createStorage(); - $storage->createSchema(); + $storage->setup(); $storage->store(JobExecution::createRoot('123', 'export')); $storage->store(JobExecution::createRoot('456', 'import')); @@ -184,7 +184,7 @@ public function testRetrieveNotFound(): void $this->expectException(JobExecutionNotFoundException::class); $storage = $this->createStorage(); - $storage->createSchema(); + $storage->setup(); $storage->store(JobExecution::createRoot('123', 'export')); $storage->retrieve('export', '456'); @@ -195,7 +195,7 @@ public function testRetrieveFailing(): void $this->expectException(JobExecutionNotFoundException::class); $storage = $this->createStorage(); - /** not calling {@see DoctrineDBALJobExecutionStorage::createSchema} will cause table to not exists */ + /** not calling {@see DoctrineDBALJobExecutionStorage::setup} will cause table to not exists */ $storage->retrieve('export', '456'); } @@ -217,7 +217,7 @@ public function testRetrieveInvalid(array $data, Throwable $error): void $data['logs'] ??= ''; $storage = $this->createStorage(); - $storage->createSchema(); + $storage->setup(); $this->connection->insert('yokai_batch_job_execution', $data); $storage->retrieve('export', '123'); } @@ -249,7 +249,7 @@ public function retrieveInvalid(): \Generator public function testList(): void { $storage = $this->createStorage(); - $storage->createSchema(); + $storage->setup(); $this->loadFixtures($storage); self::assertExecutionIds(['123'], $storage->list('export')); @@ -262,7 +262,7 @@ public function testList(): void public function testQuery(QueryBuilder $queryBuilder, array $expectedCouples): void { $storage = $this->createStorage(); - $storage->createSchema(); + $storage->setup(); $this->loadFixtures($storage); self::assertExecutions($expectedCouples, $storage->query($queryBuilder->getQuery())); diff --git a/src/batch-symfony-console/src/SetupStorageCommand.php b/src/batch-symfony-console/src/SetupStorageCommand.php new file mode 100644 index 00000000..bb2c4656 --- /dev/null +++ b/src/batch-symfony-console/src/SetupStorageCommand.php @@ -0,0 +1,53 @@ +setHelp(<<%command.name% command setups the job execution storage: + + php %command.full_name% +EOF + ) + ; + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $io = new SymfonyStyle($input, $output); + + if ($this->storage instanceof SetupableJobExecutionStorageInterface) { + $this->storage->setup(); + $io->success('The storage was set up successfully.'); + } else { + $io->note('The storage does not support setup.'); + } + + return self::SUCCESS; + } +} diff --git a/src/batch-symfony-console/tests/SetupStorageCommandTest.php b/src/batch-symfony-console/tests/SetupStorageCommandTest.php new file mode 100644 index 00000000..88b8c196 --- /dev/null +++ b/src/batch-symfony-console/tests/SetupStorageCommandTest.php @@ -0,0 +1,74 @@ +execute( + $storage = new class implements JobExecutionStorageInterface, SetupableJobExecutionStorageInterface { + public bool $wasSetup = false; + + public function setup(): void + { + $this->wasSetup = true; + } + + public function store(JobExecution $execution): void + { + } + + public function remove(JobExecution $execution): void + { + } + + public function retrieve(string $jobName, string $executionId): JobExecution + { + throw new JobExecutionNotFoundException($jobName, $executionId); + } + }, + '[OK] The storage was set up successfully.', + ); + self::assertTrue($storage->wasSetup); + } + + public function testSetupNotRequired(): void + { + $this->execute( + new class implements JobExecutionStorageInterface { + public function store(JobExecution $execution): void + { + } + + public function remove(JobExecution $execution): void + { + } + + public function retrieve(string $jobName, string $executionId): JobExecution + { + throw new JobExecutionNotFoundException($jobName, $executionId); + } + }, + '! [NOTE] The storage does not support setup.', + ); + } + + private function execute(JobExecutionStorageInterface $storage, string $expected): void + { + $tester = new CommandTester(new SetupStorageCommand($storage)); + $tester->execute([]); + $tester->assertCommandIsSuccessful(); + self::assertSame($expected, \trim($tester->getDisplay(true))); + } +} diff --git a/src/batch-symfony-framework/composer.json b/src/batch-symfony-framework/composer.json index cd6ed56c..7241cc65 100644 --- a/src/batch-symfony-framework/composer.json +++ b/src/batch-symfony-framework/composer.json @@ -14,6 +14,7 @@ "php": "^8.0", "composer-runtime-api": "^2.0", "symfony/config": "^5.0|^6.0", + "symfony/console": "^5.0|^6.0", "symfony/dependency-injection": "^5.0|^6.0", "symfony/http-kernel": "^5.0|^6.0", "symfony/framework-bundle": "^5.0|^6.0", diff --git a/src/batch-symfony-framework/src/Resources/services/symfony/console/command.xml b/src/batch-symfony-framework/src/Resources/services/symfony/console/command.xml index e9c9e004..fe2c52eb 100644 --- a/src/batch-symfony-framework/src/Resources/services/symfony/console/command.xml +++ b/src/batch-symfony-framework/src/Resources/services/symfony/console/command.xml @@ -13,5 +13,11 @@ + + + + + diff --git a/src/batch-symfony-framework/tests/CliTest.php b/src/batch-symfony-framework/tests/CliTest.php new file mode 100644 index 00000000..60f89bff --- /dev/null +++ b/src/batch-symfony-framework/tests/CliTest.php @@ -0,0 +1,26 @@ +all('yokai'), + ); + \sort($names); + self::assertSame( + [ + 'yokai:batch:run', + 'yokai:batch:setup-storage', + ], + $names, + ); + } +} diff --git a/src/batch/src/Storage/SetupableJobExecutionStorageInterface.php b/src/batch/src/Storage/SetupableJobExecutionStorageInterface.php new file mode 100644 index 00000000..93252323 --- /dev/null +++ b/src/batch/src/Storage/SetupableJobExecutionStorageInterface.php @@ -0,0 +1,16 @@ + Date: Mon, 27 Nov 2023 11:19:44 +0100 Subject: [PATCH 2/6] Fixed code style --- .../src/DoctrineDBALJobExecutionStorage.php | 5 +++-- src/batch-symfony-console/src/SetupStorageCommand.php | 6 +++--- src/batch-symfony-console/tests/SetupStorageCommandTest.php | 6 ++++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/batch-doctrine-dbal/src/DoctrineDBALJobExecutionStorage.php b/src/batch-doctrine-dbal/src/DoctrineDBALJobExecutionStorage.php index 7636951a..2b931bc3 100644 --- a/src/batch-doctrine-dbal/src/DoctrineDBALJobExecutionStorage.php +++ b/src/batch-doctrine-dbal/src/DoctrineDBALJobExecutionStorage.php @@ -26,8 +26,9 @@ * This {@see JobExecutionStorageInterface} will store * {@see JobExecution} in an SQL database using doctrine/dbal. */ -final class DoctrineDBALJobExecutionStorage implements QueryableJobExecutionStorageInterface, - SetupableJobExecutionStorageInterface +final class DoctrineDBALJobExecutionStorage implements + QueryableJobExecutionStorageInterface, + SetupableJobExecutionStorageInterface { private const DEFAULT_OPTIONS = [ 'table' => 'yokai_batch_job_execution', diff --git a/src/batch-symfony-console/src/SetupStorageCommand.php b/src/batch-symfony-console/src/SetupStorageCommand.php index bb2c4656..027c52a0 100644 --- a/src/batch-symfony-console/src/SetupStorageCommand.php +++ b/src/batch-symfony-console/src/SetupStorageCommand.php @@ -20,15 +20,15 @@ final class SetupStorageCommand extends Command { public function __construct( private JobExecutionStorageInterface $storage, - ) - { + ) { parent::__construct(); } protected function configure(): void { $this - ->setHelp(<<setHelp( + <<%command.name% command setups the job execution storage: php %command.full_name% diff --git a/src/batch-symfony-console/tests/SetupStorageCommandTest.php b/src/batch-symfony-console/tests/SetupStorageCommandTest.php index 88b8c196..d5e9e73e 100644 --- a/src/batch-symfony-console/tests/SetupStorageCommandTest.php +++ b/src/batch-symfony-console/tests/SetupStorageCommandTest.php @@ -17,7 +17,9 @@ final class SetupStorageCommandTest extends TestCase public function testSetupRequired(): void { $this->execute( - $storage = new class implements JobExecutionStorageInterface, SetupableJobExecutionStorageInterface { + $storage = new class() implements + JobExecutionStorageInterface, + SetupableJobExecutionStorageInterface { public bool $wasSetup = false; public function setup(): void @@ -46,7 +48,7 @@ public function retrieve(string $jobName, string $executionId): JobExecution public function testSetupNotRequired(): void { $this->execute( - new class implements JobExecutionStorageInterface { + new class() implements JobExecutionStorageInterface { public function store(JobExecution $execution): void { } From 9ed1e4bd09289afcee353c5f2ea472f696338a92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yann=20Eugon=C3=A9?= Date: Mon, 27 Nov 2023 11:30:49 +0100 Subject: [PATCH 3/6] Test DoctrineDBALJobExecutionStorage::createSchema deprecated method --- .../DoctrineDBALJobExecutionStorageTest.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/batch-doctrine-dbal/tests/DoctrineDBALJobExecutionStorageTest.php b/src/batch-doctrine-dbal/tests/DoctrineDBALJobExecutionStorageTest.php index c91c2891..69f7e9a9 100644 --- a/src/batch-doctrine-dbal/tests/DoctrineDBALJobExecutionStorageTest.php +++ b/src/batch-doctrine-dbal/tests/DoctrineDBALJobExecutionStorageTest.php @@ -343,6 +343,22 @@ public function queries(): Generator ]; } + public function testCreateSchemaDeprecated(): void + { + \set_error_handler( + static function () { + \restore_error_handler(); + throw new \Exception('Deprecation caught'); + }, + E_DEPRECATED, + ); + $this->expectException(\Exception::class); + $this->expectExceptionMessageMatches('Deprecation caught'); + + $storage = $this->createStorage(); + $storage->createSchema(); + } + public static function assertExecutionIds(array $ids, iterable $executions): void { $actualIds = []; From 4c3dfada2c7afa83ced0a3f2dd2fe65a324261f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yann=20Eugon=C3=A9?= Date: Mon, 27 Nov 2023 11:34:15 +0100 Subject: [PATCH 4/6] Fixed wrong usage of magic constants --- .../src/DoctrineDBALJobExecutionStorage.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/batch-doctrine-dbal/src/DoctrineDBALJobExecutionStorage.php b/src/batch-doctrine-dbal/src/DoctrineDBALJobExecutionStorage.php index 2b931bc3..6dbbcb08 100644 --- a/src/batch-doctrine-dbal/src/DoctrineDBALJobExecutionStorage.php +++ b/src/batch-doctrine-dbal/src/DoctrineDBALJobExecutionStorage.php @@ -101,8 +101,8 @@ public function createSchema(): void \sprintf( 'Since yokai/batch-doctrine-dbal 0.5.8: ' . 'Method "%s()" is deprecated and will be removed in 0.6.0. Use %s::setup() instead.', - \__METHOD__, - \__CLASS__, + __METHOD__, + __CLASS__, ), \E_USER_DEPRECATED, ); From 5e2f32e1525d6a11ae3cab61d10def2ad20e3e0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yann=20Eugon=C3=A9?= Date: Wed, 6 Dec 2023 09:25:41 +0100 Subject: [PATCH 5/6] Removed deprecation test --- .../tests/DoctrineDBALJobExecutionStorageTest.php | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/batch-doctrine-dbal/tests/DoctrineDBALJobExecutionStorageTest.php b/src/batch-doctrine-dbal/tests/DoctrineDBALJobExecutionStorageTest.php index 69f7e9a9..1387b70d 100644 --- a/src/batch-doctrine-dbal/tests/DoctrineDBALJobExecutionStorageTest.php +++ b/src/batch-doctrine-dbal/tests/DoctrineDBALJobExecutionStorageTest.php @@ -345,16 +345,6 @@ public function queries(): Generator public function testCreateSchemaDeprecated(): void { - \set_error_handler( - static function () { - \restore_error_handler(); - throw new \Exception('Deprecation caught'); - }, - E_DEPRECATED, - ); - $this->expectException(\Exception::class); - $this->expectExceptionMessageMatches('Deprecation caught'); - $storage = $this->createStorage(); $storage->createSchema(); } From 3b0e44be7153a72e83054c2bad5ea85b60cf5478 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yann=20Eugon=C3=A9?= Date: Wed, 6 Dec 2023 09:29:36 +0100 Subject: [PATCH 6/6] Proper test without deprecation --- .../tests/DoctrineDBALJobExecutionStorageTest.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/batch-doctrine-dbal/tests/DoctrineDBALJobExecutionStorageTest.php b/src/batch-doctrine-dbal/tests/DoctrineDBALJobExecutionStorageTest.php index 1387b70d..2f8ce24a 100644 --- a/src/batch-doctrine-dbal/tests/DoctrineDBALJobExecutionStorageTest.php +++ b/src/batch-doctrine-dbal/tests/DoctrineDBALJobExecutionStorageTest.php @@ -345,8 +345,10 @@ public function queries(): Generator public function testCreateSchemaDeprecated(): void { - $storage = $this->createStorage(); - $storage->createSchema(); + $schemaManager = $this->connection->getSchemaManager(); + self::assertFalse($schemaManager->tablesExist(['yokai_batch_job_execution'])); + $this->createStorage()->createSchema(); + self::assertTrue($schemaManager->tablesExist(['yokai_batch_job_execution'])); } public static function assertExecutionIds(array $ids, iterable $executions): void