From 41e0af48ec937928b3d8cf4b2e1f47e7b1b0ef04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sylvain=20Raye=CC=81?= Date: Sun, 15 Oct 2017 10:46:53 +0200 Subject: [PATCH 1/5] [ISSUE-10650][BUGFIX] Prevent running again already running cron job --- .../Observer/ProcessCronQueueObserver.php | 42 ++++++++++++++++++- .../Observer/ProcessCronQueueObserverTest.php | 12 +++--- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index f772a6c0c8493..b07a2d7c8c126 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -61,6 +61,11 @@ class ProcessCronQueueObserver implements ObserverInterface */ protected $_pendingSchedules; + /** + * @var \Magento\Cron\Model\ResourceModel\Schedule\Collection + */ + protected $_runningSchedules; + /** * @var \Magento\Cron\Model\ConfigInterface */ @@ -214,7 +219,7 @@ public function execute(\Magento\Framework\Event\Observer $observer) /** @var \Magento\Cron\Model\Schedule $schedule */ foreach ($pendingJobs as $schedule) { $jobConfig = isset($jobsRoot[$schedule->getJobCode()]) ? $jobsRoot[$schedule->getJobCode()] : null; - if (!$jobConfig) { + if (!$jobConfig || $this->isJobRunning($schedule->getJobCode())) { continue; } @@ -250,6 +255,25 @@ public function execute(\Magento\Framework\Event\Observer $observer) } } + /** + * @param $jobCode + * + * @return bool + */ + protected function isJobRunning($jobCode) + { + $runningJobs = $this->_getRunningSchedules(); + + /** @var \Magento\Cron\Model\Schedule $schedule */ + foreach ($runningJobs as $schedule) { + if ($schedule->getData('job_code') == $jobCode) { + return true; + } + } + + return false; + } + /** * Execute job by calling specific class::method * @@ -317,6 +341,22 @@ protected function _getPendingSchedules() return $this->_pendingSchedules; } + /** + * Return job collection from data base with status 'running' + * + * @return \Magento\Cron\Model\ResourceModel\Schedule\Collection + */ + protected function _getRunningSchedules() + { + if (!$this->_runningSchedules) { + $this->_runningSchedules = $this->_scheduleFactory->create()->getCollection()->addFieldToFilter( + 'status', + Schedule::STATUS_RUNNING + )->load(); + } + return $this->_runningSchedules; + } + /** * Generate cron schedule * diff --git a/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php b/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php index 0db6a598fb56f..83306ade8d11e 100644 --- a/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php +++ b/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php @@ -262,7 +262,7 @@ public function testDispatchCanNotLock() )->disableOriginalConstructor()->getMock(); $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->_collection)); $scheduleMock->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource)); - $this->_scheduleFactory->expects($this->exactly(2))->method('create')->will($this->returnValue($scheduleMock)); + $this->_scheduleFactory->expects($this->exactly(3))->method('create')->will($this->returnValue($scheduleMock)); $this->_observer->execute($this->observer); } @@ -333,7 +333,7 @@ public function testDispatchExceptionTooLate() ->disableOriginalConstructor()->getMock(); $scheduleMock->expects($this->any())->method('getCollection')->willReturn($this->_collection); $scheduleMock->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource)); - $this->_scheduleFactory->expects($this->exactly(2))->method('create')->willReturn($scheduleMock); + $this->_scheduleFactory->expects($this->exactly(3))->method('create')->willReturn($scheduleMock); $this->_observer->execute($this->observer); } @@ -388,7 +388,7 @@ public function testDispatchExceptionNoCallback() )->disableOriginalConstructor()->getMock(); $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->_collection)); $scheduleMock->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource)); - $this->_scheduleFactory->expects($this->exactly(2))->method('create')->will($this->returnValue($scheduleMock)); + $this->_scheduleFactory->expects($this->exactly(3))->method('create')->will($this->returnValue($scheduleMock)); $this->_observer->execute($this->observer); } @@ -453,7 +453,7 @@ public function testDispatchExceptionInCallback( )->disableOriginalConstructor()->getMock(); $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->_collection)); $scheduleMock->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource)); - $this->_scheduleFactory->expects($this->exactly(2))->method('create')->will($this->returnValue($scheduleMock)); + $this->_scheduleFactory->expects($this->exactly(3))->method('create')->will($this->returnValue($scheduleMock)); $this->_objectManager ->expects($this->once()) ->method('create') @@ -524,7 +524,7 @@ public function testDispatchRunJob() // cron end execute some job $schedule->expects( - $this->at(6) + $this->at(7) )->method( 'setStatus' )->with( @@ -550,7 +550,7 @@ public function testDispatchRunJob() )->disableOriginalConstructor()->getMock(); $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->_collection)); $scheduleMock->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource)); - $this->_scheduleFactory->expects($this->exactly(2))->method('create')->will($this->returnValue($scheduleMock)); + $this->_scheduleFactory->expects($this->exactly(3))->method('create')->will($this->returnValue($scheduleMock)); $testCronJob = $this->getMockBuilder('CronJob')->setMethods(['execute'])->getMock(); $testCronJob->expects($this->atLeastOnce())->method('execute')->with($schedule); From 12bd537282df1bd4b30a0bbed0092d3b32c856f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sylvain=20Ray=C3=A9?= Date: Thu, 2 Nov 2017 16:21:06 +0100 Subject: [PATCH 2/5] Update ProcessCronQueueObserver.php Remove `_` for properties for new properties / methods --- .../Cron/Observer/ProcessCronQueueObserver.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index b07a2d7c8c126..47321b175d222 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -64,7 +64,7 @@ class ProcessCronQueueObserver implements ObserverInterface /** * @var \Magento\Cron\Model\ResourceModel\Schedule\Collection */ - protected $_runningSchedules; + protected $runningSchedules; /** * @var \Magento\Cron\Model\ConfigInterface @@ -262,7 +262,7 @@ public function execute(\Magento\Framework\Event\Observer $observer) */ protected function isJobRunning($jobCode) { - $runningJobs = $this->_getRunningSchedules(); + $runningJobs = $this->getRunningSchedules(); /** @var \Magento\Cron\Model\Schedule $schedule */ foreach ($runningJobs as $schedule) { @@ -346,15 +346,15 @@ protected function _getPendingSchedules() * * @return \Magento\Cron\Model\ResourceModel\Schedule\Collection */ - protected function _getRunningSchedules() + protected function getRunningSchedules() { - if (!$this->_runningSchedules) { - $this->_runningSchedules = $this->_scheduleFactory->create()->getCollection()->addFieldToFilter( + if (!$this->runningSchedules) { + $this->runningSchedules = $this->_scheduleFactory->create()->getCollection()->addFieldToFilter( 'status', Schedule::STATUS_RUNNING )->load(); } - return $this->_runningSchedules; + return $this->runningSchedules; } /** From 110c59c86f56688f61e6e50fff89f3547574c068 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sylvain=20Ray=C3=A9?= Date: Thu, 2 Nov 2017 16:25:08 +0100 Subject: [PATCH 3/5] Update ProcessCronQueueObserver.php Make property `private` --- app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index 47321b175d222..538663d71a385 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -64,7 +64,7 @@ class ProcessCronQueueObserver implements ObserverInterface /** * @var \Magento\Cron\Model\ResourceModel\Schedule\Collection */ - protected $runningSchedules; + private $runningSchedules; /** * @var \Magento\Cron\Model\ConfigInterface From f1b6f1f9fd40fcc606dd4e7607e7b730612de455 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sylvain=20Ray=C3=A9?= Date: Thu, 2 Nov 2017 16:25:54 +0100 Subject: [PATCH 4/5] Update ProcessCronQueueObserver.php Make method `private` --- app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index 538663d71a385..bed0b10fcb471 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -260,7 +260,7 @@ public function execute(\Magento\Framework\Event\Observer $observer) * * @return bool */ - protected function isJobRunning($jobCode) + private function isJobRunning($jobCode) { $runningJobs = $this->getRunningSchedules(); From 896c8fdf667fb967c30e99723b64336c48faeeeb Mon Sep 17 00:00:00 2001 From: David Manners Date: Sat, 4 Nov 2017 10:49:36 +0000 Subject: [PATCH 5/5] Update getRunningSchedules to be private --- app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index bed0b10fcb471..c2faa39f732de 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -346,7 +346,7 @@ protected function _getPendingSchedules() * * @return \Magento\Cron\Model\ResourceModel\Schedule\Collection */ - protected function getRunningSchedules() + private function getRunningSchedules() { if (!$this->runningSchedules) { $this->runningSchedules = $this->_scheduleFactory->create()->getCollection()->addFieldToFilter(