From e63380b89b57e1d7013e311946d219ca9b722f64 Mon Sep 17 00:00:00 2001 From: Ludmil Simeonov Date: Tue, 26 Sep 2017 15:49:58 +0300 Subject: [PATCH 1/5] Fix wrong check for stopped propagation inside daemonShouldRun method. --- src/Worker.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Worker.php b/src/Worker.php index 0266dc0..e143151 100644 --- a/src/Worker.php +++ b/src/Worker.php @@ -175,7 +175,7 @@ protected function daemonShouldRun(WorkerOptions $options) { return ! (($this->manager->isDownForMaintenance() && ! $options->force) || $this->paused || - $this->until() === false); + $this->until() !== false); } /** From a61218dcb2ab1d9dd6764277c70385e09dc22f67 Mon Sep 17 00:00:00 2001 From: Ludmil Simeonov Date: Tue, 26 Sep 2017 16:39:28 +0300 Subject: [PATCH 2/5] Made some test for daemonShouldRun Made paused property a public isPaused method. --- src/Worker.php | 12 +++++- tests/WorkerTest.php | 91 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 99 insertions(+), 4 deletions(-) diff --git a/src/Worker.php b/src/Worker.php index e143151..3181538 100644 --- a/src/Worker.php +++ b/src/Worker.php @@ -171,10 +171,10 @@ protected function timeoutForJob($job, WorkerOptions $options) * * @return bool */ - protected function daemonShouldRun(WorkerOptions $options) + public function daemonShouldRun(WorkerOptions $options) { return ! (($this->manager->isDownForMaintenance() && ! $options->force) || - $this->paused || + $this->isPaused() || $this->until() !== false); } @@ -662,6 +662,14 @@ protected function until() return $this->events->dispatch(EventsList::LOOPING, new Event\Looping)->isPropagationStopped(); } + /** + * @return bool + */ + public function isPaused() + { + return $this->paused; + } + /** * Raise the failed queue job event. * diff --git a/tests/WorkerTest.php b/tests/WorkerTest.php index 495ecd1..5d2d84d 100644 --- a/tests/WorkerTest.php +++ b/tests/WorkerTest.php @@ -2,6 +2,7 @@ namespace IdeasBucket\QueueBundle; +use IdeasBucket\QueueBundle\Event\EventsList; use IdeasBucket\QueueBundle\Event\JobExceptionOccurred; use IdeasBucket\QueueBundle\Event\JobFailed; use IdeasBucket\QueueBundle\Event\JobProcessed; @@ -9,11 +10,12 @@ use IdeasBucket\QueueBundle\Exception\ErrorHandler; use IdeasBucket\QueueBundle\Exception\MaxAttemptsExceededException; use IdeasBucket\QueueBundle\Job\JobsInterface; +use Mockery as m; use PHPUnit\Framework\TestCase; use RuntimeException; -use Mockery as m; +use Symfony\Component\EventDispatcher\Event; use Symfony\Component\EventDispatcher\EventDispatcher; -use IdeasBucket\QueueBundle\Event\EventsList; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; /** * Class WorkerTest @@ -158,6 +160,91 @@ public function testJobBasedMaxRetries() $this->assertNull($job->failedWith); } + public function testDaemonShouldRunMaintenanceAndForce() + { + $manager = m::mock(Manager::class) + ->shouldReceive('isDownForMaintenance') + ->andReturn(true) + ->getMock(); + + $eventMock = m::mock(Event::class) + ->shouldReceive('isPropagationStopped') + ->andReturn(false) + ->getMock(); + + $dispatcher = m::mock(EventDispatcherInterface::class) + ->shouldReceive('dispatch') + ->andReturn($eventMock) + ->getMock(); + + $exceptionHandler = m::mock(ErrorHandler::class)->makePartial(); + + /** @var Worker|m\Mock $worker */ + $worker = m::mock(Worker::class, [$manager, $dispatcher, $exceptionHandler]) + ->makePartial() + ->shouldReceive('isPaused') + ->andReturn(false) + ->getMock(); + + $workerOptions = new WorkerOptions(); + + $bool = $worker->daemonShouldRun($workerOptions); + + $this->assertFalse($bool); + + // Now test forced + $workerOptions = new WorkerOptions(); + $workerOptions->force = true; + + $bool = $worker->daemonShouldRun($workerOptions); + $this->assertTrue($bool); + } + + public function testDaemonShouldRunUntilAndPaused(){ + $manager = m::mock(Manager::class) + ->shouldReceive('isDownForMaintenance') + ->andReturn(false) + ->getMock(); + + $eventMock = m::mock(Event::class) + ->shouldReceive('isPropagationStopped') + ->andReturnValues([true, false]) + ->getMock(); + + $dispatcher = m::mock(EventDispatcherInterface::class) + ->shouldReceive('dispatch') + ->andReturn($eventMock) + ->getMock(); + + $exceptionHandler = m::mock(ErrorHandler::class)->makePartial(); + + /** @var Worker|m\Mock $worker */ + $worker = m::mock(Worker::class, [$manager, $dispatcher, $exceptionHandler]) + ->makePartial() + ->shouldReceive('isPaused') + ->andReturn(false) + ->getMock(); + + $workerOptions = new WorkerOptions(); + + // Run once with isPropagationStopped = false + $bool = $worker->daemonShouldRun($workerOptions); + $this->assertFalse($bool); + + // Run with isPropagationStopped = true + $bool = $worker->daemonShouldRun($workerOptions); + $this->assertTrue($bool); + + $worker = m::mock(Worker::class, [$manager, $dispatcher, $exceptionHandler]) + ->makePartial() + ->shouldReceive('isPaused') + ->andReturn(true) + ->getMock(); + + $bool = $worker->daemonShouldRun($workerOptions); + $this->assertFalse($bool); + } + /** * Helpers... */ From 6c5f03fc2c7d1bd7eeb7c26ea15a95c974ef9f92 Mon Sep 17 00:00:00 2001 From: Ludmil Simeonov Date: Mon, 16 Oct 2017 10:45:00 +0300 Subject: [PATCH 3/5] Fix missing return in cache --- src/Util/CacheAdapter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Util/CacheAdapter.php b/src/Util/CacheAdapter.php index 3982dee..5fcf024 100644 --- a/src/Util/CacheAdapter.php +++ b/src/Util/CacheAdapter.php @@ -51,7 +51,7 @@ public function __construct($cache) */ public function get($key, $default = null) { - $this->cache->get($key, $default); + return $this->cache->get($key, $default); } /** From ffe09a0a5ccfbc4794795e097045046ada382f20 Mon Sep 17 00:00:00 2001 From: Ludmil Simeonov Date: Mon, 16 Oct 2017 10:53:47 +0300 Subject: [PATCH 4/5] Unit test the return bug. --- tests/Util/CacheAdapterTest.php | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 tests/Util/CacheAdapterTest.php diff --git a/tests/Util/CacheAdapterTest.php b/tests/Util/CacheAdapterTest.php new file mode 100644 index 0000000..0f8b84c --- /dev/null +++ b/tests/Util/CacheAdapterTest.php @@ -0,0 +1,31 @@ +shouldReceive('get') + ->once() + ->andReturn('test') + ->getMock(); + + $class = new CacheAdapter($cacheMock); + + + $this->assertEquals('test', $class->get('test')); + } + +} \ No newline at end of file From f2af7b241a488b6a94e6311d641262b88165f025 Mon Sep 17 00:00:00 2001 From: Ludmil Simeonov Date: Thu, 1 Feb 2018 15:38:43 +0200 Subject: [PATCH 5/5] Make sure we provide array inside set payload so doctrine will not double encode the payload. --- src/Command/Stubs/Orm/FailedRepository.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Command/Stubs/Orm/FailedRepository.txt b/src/Command/Stubs/Orm/FailedRepository.txt index b68b196..7faa244 100644 --- a/src/Command/Stubs/Orm/FailedRepository.txt +++ b/src/Command/Stubs/Orm/FailedRepository.txt @@ -21,10 +21,12 @@ class {{className}}Repository extends EntityRepository implements RepositoryInt /** @var EntityInterface $entity */ $entity = new {{className}}; + $payload = json_decode($rawBody, true); + $entity->setConnection($connectionName) ->setQueue($queue) ->setException($exception->getMessage()) - ->setPayload($rawBody) + ->setPayload($payload) ->setFailedAt(new \DateTime); $em = $this->getEntityManager();