diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index c8dcd8049fbe8..6008d4ebce218 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -3,16 +3,15 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ - /** * Handling cron jobs */ namespace Magento\Cron\Observer; +use Magento\Cron\Model\Schedule; use Magento\Framework\App\State; use Magento\Framework\Console\Cli; use Magento\Framework\Event\ObserverInterface; -use \Magento\Cron\Model\Schedule; use Magento\Framework\Profiler\Driver\Standard\Stat; use Magento\Framework\Profiler\Driver\Standard\StatFactory; @@ -201,7 +200,6 @@ public function __construct( */ public function execute(\Magento\Framework\Event\Observer $observer) { - $currentTime = $this->dateTime->gmtTimestamp(); $jobGroupsRoot = $this->_config->getJobs(); // sort jobs groups to start from used in separated process @@ -255,7 +253,6 @@ function ($groupId) use ($currentTime, $jobsRoot) { */ private function lockGroup($groupId, callable $callback) { - if (!$this->lockManager->lock(self::LOCK_PREFIX . $groupId, self::LOCK_TIMEOUT)) { $this->logger->warning( sprintf( @@ -290,17 +287,20 @@ protected function _runJob($scheduledTime, $currentTime, $jobConfig, $schedule, $scheduleLifetime = $scheduleLifetime * self::SECONDS_IN_MINUTE; if ($scheduledTime < $currentTime - $scheduleLifetime) { $schedule->setStatus(Schedule::STATUS_MISSED); + // phpcs:ignore Magento2.Exceptions.DirectThrow throw new \Exception(sprintf('Cron Job %s is missed at %s', $jobCode, $schedule->getScheduledAt())); } if (!isset($jobConfig['instance'], $jobConfig['method'])) { $schedule->setStatus(Schedule::STATUS_ERROR); - throw new \Exception('No callbacks found'); + // phpcs:ignore Magento2.Exceptions.DirectThrow + throw new \Exception(sprintf('No callbacks found for cron job %s', $jobCode)); } $model = $this->_objectManager->create($jobConfig['instance']); $callback = [$model, $jobConfig['method']]; if (!is_callable($callback)) { $schedule->setStatus(Schedule::STATUS_ERROR); + // phpcs:ignore Magento2.Exceptions.DirectThrow throw new \Exception( sprintf('Invalid callback: %s::%s can\'t be called', $jobConfig['instance'], $jobConfig['method']) ); @@ -311,15 +311,18 @@ protected function _runJob($scheduledTime, $currentTime, $jobConfig, $schedule, $this->startProfiling(); try { $this->logger->info(sprintf('Cron Job %s is run', $jobCode)); + //phpcs:ignore Magento2.Functions.DiscouragedFunction call_user_func_array($callback, [$schedule]); } catch (\Throwable $e) { $schedule->setStatus(Schedule::STATUS_ERROR); - $this->logger->error(sprintf( - 'Cron Job %s has an error: %s. Statistics: %s', - $jobCode, - $e->getMessage(), - $this->getProfilingStat() - )); + $this->logger->error( + sprintf( + 'Cron Job %s has an error: %s. Statistics: %s', + $jobCode, + $e->getMessage(), + $this->getProfilingStat() + ) + ); if (!$e instanceof \Exception) { $e = new \RuntimeException( 'Error when running a cron job', @@ -332,16 +335,22 @@ protected function _runJob($scheduledTime, $currentTime, $jobConfig, $schedule, $this->stopProfiling(); } - $schedule->setStatus(Schedule::STATUS_SUCCESS)->setFinishedAt(strftime( - '%Y-%m-%d %H:%M:%S', - $this->dateTime->gmtTimestamp() - )); + $schedule->setStatus( + Schedule::STATUS_SUCCESS + )->setFinishedAt( + strftime( + '%Y-%m-%d %H:%M:%S', + $this->dateTime->gmtTimestamp() + ) + ); - $this->logger->info(sprintf( - 'Cron Job %s is successfully finished. Statistics: %s', - $jobCode, - $this->getProfilingStat() - )); + $this->logger->info( + sprintf( + 'Cron Job %s is successfully finished. Statistics: %s', + $jobCode, + $this->getProfilingStat() + ) + ); } /** @@ -391,6 +400,28 @@ private function getPendingSchedules($groupId) return $pendingJobs; } + /** + * Return job collection from database with status 'pending', 'running' or 'success' + * + * @param string $groupId + * @return \Magento\Framework\Model\ResourceModel\Db\Collection\AbstractCollection + */ + private function getNonExitedSchedules($groupId) + { + $jobs = $this->_config->getJobs(); + $pendingJobs = $this->_scheduleFactory->create()->getCollection(); + $pendingJobs->addFieldToFilter( + 'status', + [ + 'in' => [ + Schedule::STATUS_PENDING, Schedule::STATUS_RUNNING, Schedule::STATUS_SUCCESS + ] + ] + ); + $pendingJobs->addFieldToFilter('job_code', ['in' => array_keys($jobs[$groupId])]); + return $pendingJobs; + } + /** * Generate cron schedule * @@ -422,7 +453,7 @@ private function generateSchedules($groupId) null ); - $schedules = $this->getPendingSchedules($groupId); + $schedules = $this->getNonExitedSchedules($groupId); $exists = []; /** @var Schedule $schedule */ foreach ($schedules as $schedule) { @@ -653,11 +684,14 @@ private function cleanupScheduleMismatches() /** @var \Magento\Cron\Model\ResourceModel\Schedule $scheduleResource */ $scheduleResource = $this->_scheduleFactory->create()->getResource(); foreach ($this->invalid as $jobCode => $scheduledAtList) { - $scheduleResource->getConnection()->delete($scheduleResource->getMainTable(), [ - 'status = ?' => Schedule::STATUS_PENDING, - 'job_code = ?' => $jobCode, - 'scheduled_at in (?)' => $scheduledAtList, - ]); + $scheduleResource->getConnection()->delete( + $scheduleResource->getMainTable(), + [ + 'status = ?' => Schedule::STATUS_PENDING, + 'job_code = ?' => $jobCode, + 'scheduled_at in (?)' => $scheduledAtList, + ] + ); } return $this; } diff --git a/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php b/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php index d14249e6b0e57..f69cb13c17e47 100644 --- a/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php +++ b/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php @@ -354,7 +354,8 @@ public function testDispatchExceptionTooLate() */ public function testDispatchExceptionNoCallback() { - $exceptionMessage = 'No callbacks found'; + $jobCode = 'test_job1'; + $exceptionMessage = sprintf('No callbacks found for cron job %s', $jobCode); $exception = new \Exception(__($exceptionMessage)); $dateScheduledAt = date('Y-m-d H:i:s', $this->time - 86400); @@ -363,7 +364,7 @@ public function testDispatchExceptionNoCallback() )->setMethods( ['getJobCode', 'tryLockJob', 'getScheduledAt', 'save', 'setStatus', 'setMessages', '__wakeup', 'getStatus'] )->disableOriginalConstructor()->getMock(); - $schedule->expects($this->any())->method('getJobCode')->will($this->returnValue('test_job1')); + $schedule->expects($this->any())->method('getJobCode')->will($this->returnValue($jobCode)); $schedule->expects($this->once())->method('getScheduledAt')->will($this->returnValue($dateScheduledAt)); $schedule->expects($this->once())->method('tryLockJob')->will($this->returnValue(true)); $schedule->expects( @@ -383,7 +384,7 @@ public function testDispatchExceptionNoCallback() $this->loggerMock->expects($this->once())->method('critical')->with($exception); - $jobConfig = ['test_group' => ['test_job1' => ['instance' => 'Some_Class']]]; + $jobConfig = ['test_group' => [$jobCode => ['instance' => 'Some_Class']]]; $this->_config->expects($this->exactly(2))->method('getJobs')->will($this->returnValue($jobConfig));