From 2f78c77248005a9795cede8465eb2a7f98d9a613 Mon Sep 17 00:00:00 2001 From: viktorprogger Date: Tue, 9 Jun 2020 18:43:56 +0300 Subject: [PATCH 1/6] More tests and bugfixes --- composer.json | 15 +++++-- config/common.php | 2 +- phpunit.xml.dist | 6 +++ src/Queue.php | 5 ++- tests/App/DelayableJob.php | 14 ++++++ tests/App/EventManager.php | 20 +++++++++ tests/App/PrioritizedJob.php | 14 ++++++ tests/App/PriorityJob.php | 38 ---------------- tests/App/RetryJob.php | 50 --------------------- tests/App/RetryableJob.php | 31 +++++++++++++ tests/App/config/console.php | 3 ++ tests/App/config/main.php | 4 -- tests/TestCase.php | 5 ++- tests/unit/QueueTest.php | 84 ++++++++++++++++++++++++++++++++++++ tests/unit/WorkerTest.php | 1 - 15 files changed, 191 insertions(+), 101 deletions(-) create mode 100644 tests/App/DelayableJob.php create mode 100644 tests/App/EventManager.php create mode 100644 tests/App/PrioritizedJob.php delete mode 100644 tests/App/PriorityJob.php delete mode 100644 tests/App/RetryJob.php create mode 100644 tests/App/RetryableJob.php delete mode 100644 tests/App/config/main.php create mode 100644 tests/unit/QueueTest.php diff --git a/composer.json b/composer.json index 876d68fd..a6b4744f 100644 --- a/composer.json +++ b/composer.json @@ -36,7 +36,8 @@ "phpunit/phpunit": "^9.0", "yiisoft/composer-config-plugin": "^1.0@dev", "yiisoft/di": "^3.0@dev", - "yiisoft/log": "^3.0@dev" + "yiisoft/log": "^3.0@dev", + "yiisoft/yii-queue-driver-synchronous": "dev-master" }, "suggest": { "ext-pcntl": "Need for process signals." @@ -66,12 +67,18 @@ "tests-app": [ "$common", "$console", - "tests/App/config/main.php", "tests/App/config/console.php" - ] + ], + "events-console": "config/events-console.php" } }, "config": { "sort-packages": true - } + }, + "repositories": [ + { + "type": "vcs", + "url": "https://github.com/yiisoft/yii-queue-synchronous" + } + ] } diff --git a/config/common.php b/config/common.php index bf333217..17a445d4 100644 --- a/config/common.php +++ b/config/common.php @@ -15,6 +15,6 @@ EventDispatcherInterface::class => Dispatcher::class, WorkerInterface::class => QueueWorker::class, ListenerProviderInterface::class => Provider::class, - ContainerInterface::class => Container::class, + ContainerInterface::class => fn(ContainerInterface $container) => $container, LoopInterface::class => SignalLoop::class ]; diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 662e1775..337e4813 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -10,4 +10,10 @@ ./tests/unit + + + + ./src + + diff --git a/src/Queue.php b/src/Queue.php index e2d64a18..add5b5cd 100644 --- a/src/Queue.php +++ b/src/Queue.php @@ -59,6 +59,7 @@ public function jobRetry(JobFailure $event): void && $event->getQueue() === $this && $job->canRetry($event->getException()) ) { + $event->preventThrowing(); $this->logger->debug('Retrying job "{job}".', ['job' => get_class($job)]); $job->retry(); $this->push($job); @@ -70,9 +71,9 @@ public function jobRetry(JobFailure $event): void * * @param JobInterface|mixed $job * - * @return string|null id of a job message + * @return string id of a job message */ - public function push(JobInterface $job): ?string + public function push(JobInterface $job): string { $this->logger->debug('Preparing to push job "{job}".', ['job' => get_class($job)]); $event = new BeforePush($this, $job); diff --git a/tests/App/DelayableJob.php b/tests/App/DelayableJob.php new file mode 100644 index 00000000..1d82bc49 --- /dev/null +++ b/tests/App/DelayableJob.php @@ -0,0 +1,14 @@ + - */ -class PriorityJob extends BaseObject implements JobInterface -{ - public $number; - - public function __construct($number) - { - $this->number = $number; - } - - public function execute($queue) - { - file_put_contents(self::getFileName(), $this->number, FILE_APPEND); - } - - public static function getFileName() - { - return Yii::getAlias('@runtime/job-priority.log'); - } -} diff --git a/tests/App/RetryJob.php b/tests/App/RetryJob.php deleted file mode 100644 index 6b6edeb8..00000000 --- a/tests/App/RetryJob.php +++ /dev/null @@ -1,50 +0,0 @@ - - */ -class RetryJob extends BaseObject implements RetryableJobInterface -{ - public $uid; - - public function __construct($uid) - { - $this->uid = $uid; - } - - public function execute($queue) - { - file_put_contents($this->getFileName(), 'a', FILE_APPEND); - - throw new \Exception('Planned error.'); - } - - public function getFileName() - { - return Yii::getAlias("@runtime/job-{$this->uid}.lock"); - } - - public function getTtr(): int - { - return 2; - } - - public function canRetry($attempt, $error): bool - { - return $attempt < 2; - } -} diff --git a/tests/App/RetryableJob.php b/tests/App/RetryableJob.php new file mode 100644 index 00000000..d17ea601 --- /dev/null +++ b/tests/App/RetryableJob.php @@ -0,0 +1,31 @@ +attemptsMax = $attemptsMax; + } + + public function getTtr(): int + { + return 1; + } + + public function execute(): void + { + if ($this->canRetry()) { + throw new RuntimeException('Test exception'); + } + + $this->executed = true; + } +} diff --git a/tests/App/config/console.php b/tests/App/config/console.php index 66b70c61..4bb3727e 100644 --- a/tests/App/config/console.php +++ b/tests/App/config/console.php @@ -5,6 +5,8 @@ use Psr\Log\NullLogger; use Symfony\Component\Console\CommandLoader\ContainerCommandLoader; use Yiisoft\Yii\Console\Application; +use Yiisoft\Yii\Queue\Driver\SynchronousDriver; +use Yiisoft\Yii\Queue\DriverInterface; use Yiisoft\Yii\Queue\Tests\App\Benchmark\Controller; return [ @@ -23,4 +25,5 @@ return $app; }, LoggerInterface::class => NullLogger::class, + DriverInterface::class => SynchronousDriver::class, ]; diff --git a/tests/App/config/main.php b/tests/App/config/main.php deleted file mode 100644 index b6251283..00000000 --- a/tests/App/config/main.php +++ /dev/null @@ -1,4 +0,0 @@ -container = new Container(require Builder::path('tests-app')); + $eventConfigurator = $this->container->get(EventConfigurator::class); + $eventConfigurator->registerListeners(require Builder::path('events-console')); } } diff --git a/tests/unit/QueueTest.php b/tests/unit/QueueTest.php new file mode 100644 index 00000000..ee2df548 --- /dev/null +++ b/tests/unit/QueueTest.php @@ -0,0 +1,84 @@ +eventManager = $this->createMock(EventManager::class); + + $configurator = $this->container->get(EventConfigurator::class); + $configurator->registerListeners([BeforePush::class => [[$this->eventManager, 'beforePushHandler']]]); + $configurator->registerListeners([AfterPush::class => [[$this->eventManager, 'afterPushHandler']]]); + $configurator->registerListeners([BeforeExecution::class => [[$this->eventManager, 'beforeExecutionHandler']]]); + $configurator->registerListeners([AfterExecution::class => [[$this->eventManager, 'afterExecutionHandler']]]); + $configurator->registerListeners([JobFailure::class => [[$this->eventManager, 'jobFailureHandler']]]); + } + + public function testPushSuccessful(): void + { + $this->eventManager->expects(self::once())->method('beforePushHandler'); + $this->eventManager->expects(self::once())->method('afterPushHandler'); + + $queue = $this->container->get(Queue::class); + $job = $this->container->get(SimpleJob::class); + $id = $queue->push($job); + + $this->assertNotEmpty($id, 'Pushed message should has an id'); + } + + public function testPushNotSuccessful(): void + { + $this->expectException(JobNotSupportedException::class); + $this->eventManager->expects(self::once())->method('beforePushHandler'); + $this->eventManager->expects(self::never())->method('afterPushHandler'); + + $queue = $this->container->get(Queue::class); + $job = $this->container->get(DelayableJob::class); + $queue->push($job); + } + + public function testJobRetry(): void + { + $this->eventManager->expects(self::once())->method('jobFailureHandler'); + + $queue = $this->container->get(Queue::class); + $job = $this->container->get(RetryableJob::class); + $queue->push($job); + $queue->run(); + + $this->assertTrue($job->executed); + } + + public function testStatus(): void + { + $queue = $this->container->get(Queue::class); + $job = $this->container->get(SimpleJob::class); + $id = $queue->push($job); + + $queue->status($id); + } +} diff --git a/tests/unit/WorkerTest.php b/tests/unit/WorkerTest.php index 33f40018..25a506b0 100644 --- a/tests/unit/WorkerTest.php +++ b/tests/unit/WorkerTest.php @@ -49,7 +49,6 @@ public function testJobExecuted(): void public function testJobNotExecuted(): void { $handler = fn (BeforeExecution $event) => $event->stopExecution(); - /** @var EventConfigurator $configurator */ $configurator = $this->container->get(EventConfigurator::class); $configurator->registerListeners([BeforeExecution::class => [$handler]]); From 4d5f5b04f63406437352004740f9ea016fbb08d4 Mon Sep 17 00:00:00 2001 From: viktorprogger Date: Tue, 9 Jun 2020 18:47:56 +0300 Subject: [PATCH 2/6] Code style --- config/common.php | 2 +- src/Job/RetryableJobInterface.php | 8 ++------ tests/App/DelayableJob.php | 1 + tests/App/PrioritizedJob.php | 1 + tests/App/RetryableJob.php | 1 + tests/App/SimpleJob.php | 8 ++------ tests/unit/QueueTest.php | 1 + 7 files changed, 9 insertions(+), 13 deletions(-) diff --git a/config/common.php b/config/common.php index 17a445d4..f8db1f67 100644 --- a/config/common.php +++ b/config/common.php @@ -15,6 +15,6 @@ EventDispatcherInterface::class => Dispatcher::class, WorkerInterface::class => QueueWorker::class, ListenerProviderInterface::class => Provider::class, - ContainerInterface::class => fn(ContainerInterface $container) => $container, + ContainerInterface::class => fn (ContainerInterface $container) => $container, LoopInterface::class => SignalLoop::class ]; diff --git a/src/Job/RetryableJobInterface.php b/src/Job/RetryableJobInterface.php index 33c444a0..e57e12bf 100644 --- a/src/Job/RetryableJobInterface.php +++ b/src/Job/RetryableJobInterface.php @@ -1,10 +1,6 @@ Date: Wed, 10 Jun 2020 10:37:39 +0300 Subject: [PATCH 3/6] Move synchronous driver to this package --- composer.json | 11 +-- src/{ => Driver}/DriverInterface.php | 3 +- src/Driver/SynchronousDriver.php | 100 +++++++++++++++++++++ src/Exception/JobNotSupportedException.php | 2 +- src/Queue.php | 1 + tests/App/config/console.php | 2 +- 6 files changed, 107 insertions(+), 12 deletions(-) rename src/{ => Driver}/DriverInterface.php (95%) create mode 100644 src/Driver/SynchronousDriver.php diff --git a/composer.json b/composer.json index a6b4744f..0491ad12 100644 --- a/composer.json +++ b/composer.json @@ -36,8 +36,7 @@ "phpunit/phpunit": "^9.0", "yiisoft/composer-config-plugin": "^1.0@dev", "yiisoft/di": "^3.0@dev", - "yiisoft/log": "^3.0@dev", - "yiisoft/yii-queue-driver-synchronous": "dev-master" + "yiisoft/log": "^3.0@dev" }, "suggest": { "ext-pcntl": "Need for process signals." @@ -74,11 +73,5 @@ }, "config": { "sort-packages": true - }, - "repositories": [ - { - "type": "vcs", - "url": "https://github.com/yiisoft/yii-queue-synchronous" - } - ] + } } diff --git a/src/DriverInterface.php b/src/Driver/DriverInterface.php similarity index 95% rename from src/DriverInterface.php rename to src/Driver/DriverInterface.php index 3f46b7e2..38ef9340 100644 --- a/src/DriverInterface.php +++ b/src/Driver/DriverInterface.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Yiisoft\Yii\Queue; +namespace Yiisoft\Yii\Queue\Driver; use InvalidArgumentException; use Yiisoft\Yii\Queue\Enum\JobStatus; @@ -10,6 +10,7 @@ use Yiisoft\Yii\Queue\Job\JobInterface; use Yiisoft\Yii\Queue\Job\PrioritisedJobInterface; use Yiisoft\Yii\Queue\Job\RetryableJobInterface; +use Yiisoft\Yii\Queue\MessageInterface; interface DriverInterface { diff --git a/src/Driver/SynchronousDriver.php b/src/Driver/SynchronousDriver.php new file mode 100644 index 00000000..cf794828 --- /dev/null +++ b/src/Driver/SynchronousDriver.php @@ -0,0 +1,100 @@ +loop = $loop; + $this->worker = $worker; + } + + public function __destruct() + { + $this->run([$this->worker, 'process']); + } + + public function nextMessage(): ?MessageInterface + { + $message = null; + + if (isset($this->messages[$this->current])) { + $message = $this->messages[$this->current]; + unset($this->messages[$this->current]); + $this->current++; + } + + return $message; + } + + public function status(string $id): JobStatus + { + $id = (int) $id; + + if ($id < 0) { + throw new InvalidArgumentException('This driver ids starts with 0'); + } + + if ($id < $this->current) { + return JobStatus::done(); + } + + if (isset($this->messages[$id])) { + return JobStatus::waiting(); + } + + throw new InvalidArgumentException('There is no job with the given id.'); + } + + public function push(JobInterface $job): MessageInterface + { + $key = count($this->messages) + $this->current; + $message = new Message((string) $key, $job); + $this->messages[] = $message; + + return $message; + } + + public function subscribe(callable $handler): void + { + $this->run($handler); + } + + public function canPush(JobInterface $job): bool + { + return !($job instanceof DelayableJobInterface || $job instanceof PrioritisedJobInterface); + } + + public function setQueue(Queue $queue): void + { + $this->queue = $queue; + } + + private function run(callable $handler): void + { + while ($this->loop->canContinue() && $message = $this->nextMessage()) { + $handler($message, $this->queue); + } + } +} diff --git a/src/Exception/JobNotSupportedException.php b/src/Exception/JobNotSupportedException.php index 180477aa..5ab673d0 100644 --- a/src/Exception/JobNotSupportedException.php +++ b/src/Exception/JobNotSupportedException.php @@ -7,7 +7,7 @@ use Throwable; use UnexpectedValueException; use Yiisoft\FriendlyException\FriendlyExceptionInterface; -use Yiisoft\Yii\Queue\DriverInterface; +use Yiisoft\Yii\Queue\Driver\DriverInterface; use Yiisoft\Yii\Queue\Job\DelayableJobInterface; use Yiisoft\Yii\Queue\Job\JobInterface; use Yiisoft\Yii\Queue\Job\PrioritisedJobInterface; diff --git a/src/Queue.php b/src/Queue.php index add5b5cd..490f6398 100644 --- a/src/Queue.php +++ b/src/Queue.php @@ -7,6 +7,7 @@ use Psr\Log\LoggerInterface; use Yiisoft\EventDispatcher\Provider\Provider; use Yiisoft\Yii\Queue\Cli\LoopInterface; +use Yiisoft\Yii\Queue\Driver\DriverInterface; use Yiisoft\Yii\Queue\Enum\JobStatus; use Yiisoft\Yii\Queue\Event\AfterPush; use Yiisoft\Yii\Queue\Event\BeforePush; diff --git a/tests/App/config/console.php b/tests/App/config/console.php index 4bb3727e..041ec423 100644 --- a/tests/App/config/console.php +++ b/tests/App/config/console.php @@ -5,8 +5,8 @@ use Psr\Log\NullLogger; use Symfony\Component\Console\CommandLoader\ContainerCommandLoader; use Yiisoft\Yii\Console\Application; +use Yiisoft\Yii\Queue\Driver\DriverInterface; use Yiisoft\Yii\Queue\Driver\SynchronousDriver; -use Yiisoft\Yii\Queue\DriverInterface; use Yiisoft\Yii\Queue\Tests\App\Benchmark\Controller; return [ From 0b7cc9166f31ae5f672790d1501fec50d6902269 Mon Sep 17 00:00:00 2001 From: viktorprogger Date: Wed, 10 Jun 2020 10:37:50 +0300 Subject: [PATCH 4/6] Fill in the status test --- tests/unit/QueueTest.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/unit/QueueTest.php b/tests/unit/QueueTest.php index 827d9f68..024c9d06 100644 --- a/tests/unit/QueueTest.php +++ b/tests/unit/QueueTest.php @@ -48,7 +48,7 @@ public function testPushSuccessful(): void $job = $this->container->get(SimpleJob::class); $id = $queue->push($job); - $this->assertNotEmpty($id, 'Pushed message should has an id'); + $this->assertNotEquals('', $id, 'Pushed message should has an id'); } public function testPushNotSuccessful(): void @@ -80,6 +80,11 @@ public function testStatus(): void $job = $this->container->get(SimpleJob::class); $id = $queue->push($job); - $queue->status($id); + $status = $queue->status($id); + $this->assertEquals(true, $status->isWaiting()); + + $queue->run(); + $status = $queue->status($id); + $this->assertEquals(true, $status->isDone()); } } From 73aa3e13c3af8773f813a8e528b4f09ed20c3fac Mon Sep 17 00:00:00 2001 From: viktorprogger Date: Wed, 10 Jun 2020 11:07:01 +0300 Subject: [PATCH 5/6] Improve tests --- tests/unit/QueueTest.php | 10 +++++ tests/unit/SynchronousDriverTest.php | 62 ++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 tests/unit/SynchronousDriverTest.php diff --git a/tests/unit/QueueTest.php b/tests/unit/QueueTest.php index 024c9d06..019a062d 100644 --- a/tests/unit/QueueTest.php +++ b/tests/unit/QueueTest.php @@ -43,6 +43,9 @@ public function testPushSuccessful(): void { $this->eventManager->expects(self::once())->method('beforePushHandler'); $this->eventManager->expects(self::once())->method('afterPushHandler'); + $this->eventManager->expects(self::never())->method('beforeExecutionHandler'); + $this->eventManager->expects(self::never())->method('afterExecutionHandler'); + $this->eventManager->expects(self::never())->method('jobFailureHandler'); $queue = $this->container->get(Queue::class); $job = $this->container->get(SimpleJob::class); @@ -56,6 +59,9 @@ public function testPushNotSuccessful(): void $this->expectException(JobNotSupportedException::class); $this->eventManager->expects(self::once())->method('beforePushHandler'); $this->eventManager->expects(self::never())->method('afterPushHandler'); + $this->eventManager->expects(self::never())->method('beforeExecutionHandler'); + $this->eventManager->expects(self::never())->method('afterExecutionHandler'); + $this->eventManager->expects(self::never())->method('jobFailureHandler'); $queue = $this->container->get(Queue::class); $job = $this->container->get(DelayableJob::class); @@ -64,6 +70,10 @@ public function testPushNotSuccessful(): void public function testJobRetry(): void { + $this->eventManager->expects(self::exactly(2))->method('beforePushHandler'); + $this->eventManager->expects(self::exactly(2))->method('afterPushHandler'); + $this->eventManager->expects(self::exactly(2))->method('beforeExecutionHandler'); + $this->eventManager->expects(self::once())->method('afterExecutionHandler'); $this->eventManager->expects(self::once())->method('jobFailureHandler'); $queue = $this->container->get(Queue::class); diff --git a/tests/unit/SynchronousDriverTest.php b/tests/unit/SynchronousDriverTest.php new file mode 100644 index 00000000..b1dfb9ba --- /dev/null +++ b/tests/unit/SynchronousDriverTest.php @@ -0,0 +1,62 @@ +container->get(Queue::class); + $job = $this->container->get($class); + + if (!$available) { + $this->expectException(JobNotSupportedException::class); + } + + $id = $queue->push($job); + + if ($available) { + $this->assertTrue($id >= 0); + } + } + + public static function getJobTypes(): array + { + return [ + 'Simple job' => [ + SimpleJob::class, + true, + ], + DelayableJobInterface::class => [ + DelayableJob::class, + false, + ], + PrioritisedJobInterface::class => [ + PrioritizedJob::class, + false, + ], + RetryableJobInterface::class => [ + RetryableJob::class, + true, + ], + ]; + } +} From 82699e084c34fe42472c64765494a874b8dbd780 Mon Sep 17 00:00:00 2001 From: viktorprogger Date: Wed, 10 Jun 2020 14:58:43 +0300 Subject: [PATCH 6/6] Fix php version --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2dc627e1..61a36c51 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -20,7 +20,7 @@ jobs: - windows-latest php-version: - - "7.4" + - "7.4.5" - "8.0" steps: