From a3ca4371fbfbe399d15009e1569b00e030205420 Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Wed, 14 Jun 2017 15:24:59 +0600 Subject: [PATCH 01/15] Clean up the cron observer before adding edits --- .../Observer/ProcessCronQueueObserver.php | 104 +++++++++--------- 1 file changed, 52 insertions(+), 52 deletions(-) diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index 1e494d59046ef..3946b0f224746 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -59,42 +59,42 @@ class ProcessCronQueueObserver implements ObserverInterface /** * @var \Magento\Cron\Model\ResourceModel\Schedule\Collection */ - protected $_pendingSchedules; + protected $pendingSchedules; /** * @var \Magento\Cron\Model\ConfigInterface */ - protected $_config; + protected $config; /** * @var \Magento\Framework\App\ObjectManager */ - protected $_objectManager; + protected $objectManager; /** * @var \Magento\Framework\App\CacheInterface */ - protected $_cache; + protected $cache; /** * @var \Magento\Framework\App\Config\ScopeConfigInterface */ - protected $_scopeConfig; + protected $scopeConfig; /** * @var ScheduleFactory */ - protected $_scheduleFactory; + protected $scheduleFactory; /** * @var \Magento\Framework\App\Console\Request */ - protected $_request; + protected $request; /** * @var \Magento\Framework\ShellInterface */ - protected $_shell; + protected $shell; /** * @var \Magento\Framework\Stdlib\DateTime\TimezoneInterface @@ -143,13 +143,13 @@ public function __construct( \Psr\Log\LoggerInterface $logger, \Magento\Framework\App\State $state ) { - $this->_objectManager = $objectManager; - $this->_scheduleFactory = $scheduleFactory; - $this->_cache = $cache; - $this->_config = $config; - $this->_scopeConfig = $scopeConfig; - $this->_request = $request; - $this->_shell = $shell; + $this->objectManager = $objectManager; + $this->scheduleFactory = $scheduleFactory; + $this->cache = $cache; + $this->config = $config; + $this->scopeConfig = $scopeConfig; + $this->request = $request; + $this->shell = $shell; $this->timezone = $timezone; $this->phpExecutableFinder = $phpExecutableFinderFactory->create(); $this->logger = $logger; @@ -169,25 +169,25 @@ public function __construct( */ public function execute(\Magento\Framework\Event\Observer $observer) { - $pendingJobs = $this->_getPendingSchedules(); + $pendingJobs = $this->getPendingSchedules(); $currentTime = $this->timezone->scopeTimeStamp(); - $jobGroupsRoot = $this->_config->getJobs(); + $jobGroupsRoot = $this->config->getJobs(); $phpPath = $this->phpExecutableFinder->find() ?: 'php'; foreach ($jobGroupsRoot as $groupId => $jobsRoot) { - if ($this->_request->getParam('group') !== null - && $this->_request->getParam('group') !== '\'' . ($groupId) . '\'' - && $this->_request->getParam('group') !== $groupId) { + if ($this->request->getParam('group') !== null + && $this->request->getParam('group') !== '\'' . ($groupId) . '\'' + && $this->request->getParam('group') !== $groupId) { continue; } - if (($this->_request->getParam(self::STANDALONE_PROCESS_STARTED) !== '1') && ( - $this->_scopeConfig->getValue( + if (($this->request->getParam(self::STANDALONE_PROCESS_STARTED) !== '1') && ( + $this->scopeConfig->getValue( 'system/cron/' . $groupId . '/use_separate_process', \Magento\Store\Model\ScopeInterface::SCOPE_STORE ) == 1 )) { - $this->_shell->execute( + $this->shell->execute( $phpPath . ' %s cron:run --group=' . $groupId . ' --' . Cli::INPUT_KEY_BOOTSTRAP . '=' . self::STANDALONE_PROCESS_STARTED . '=1', [ @@ -210,7 +210,7 @@ public function execute(\Magento\Framework\Event\Observer $observer) try { if ($schedule->tryLockJob()) { - $this->_runJob($scheduledTime, $currentTime, $jobConfig, $schedule, $groupId); + $this->runJob($scheduledTime, $currentTime, $jobConfig, $schedule, $groupId); } } catch (\Exception $e) { $schedule->setMessages($e->getMessage()); @@ -233,8 +233,8 @@ public function execute(\Magento\Framework\Event\Observer $observer) $schedule->save(); } - $this->_generate($groupId); - $this->_cleanup($groupId); + $this->generate($groupId); + $this->cleanup($groupId); } } @@ -249,9 +249,9 @@ public function execute(\Magento\Framework\Event\Observer $observer) * @return void * @throws \Exception */ - protected function _runJob($scheduledTime, $currentTime, $jobConfig, $schedule, $groupId) + protected function runJob($scheduledTime, $currentTime, $jobConfig, $schedule, $groupId) { - $scheduleLifetime = (int)$this->_scopeConfig->getValue( + $scheduleLifetime = (int)$this->scopeConfig->getValue( 'system/cron/' . $groupId . '/' . self::XML_PATH_SCHEDULE_LIFETIME, \Magento\Store\Model\ScopeInterface::SCOPE_STORE ); @@ -265,7 +265,7 @@ protected function _runJob($scheduledTime, $currentTime, $jobConfig, $schedule, $schedule->setStatus(Schedule::STATUS_ERROR); throw new \Exception('No callbacks found'); } - $model = $this->_objectManager->create($jobConfig['instance']); + $model = $this->objectManager->create($jobConfig['instance']); $callback = [$model, $jobConfig['method']]; if (!is_callable($callback)) { $schedule->setStatus(Schedule::STATUS_ERROR); @@ -294,15 +294,15 @@ protected function _runJob($scheduledTime, $currentTime, $jobConfig, $schedule, * * @return \Magento\Cron\Model\ResourceModel\Schedule\Collection */ - protected function _getPendingSchedules() + protected function getPendingSchedules() { - if (!$this->_pendingSchedules) { - $this->_pendingSchedules = $this->_scheduleFactory->create()->getCollection()->addFieldToFilter( + if (!$this->pendingSchedules) { + $this->pendingSchedules = $this->scheduleFactory->create()->getCollection()->addFieldToFilter( 'status', Schedule::STATUS_PENDING )->load(); } - return $this->_pendingSchedules; + return $this->pendingSchedules; } /** @@ -311,13 +311,13 @@ protected function _getPendingSchedules() * @param string $groupId * @return $this */ - protected function _generate($groupId) + protected function generate($groupId) { /** * check if schedule generation is needed */ - $lastRun = (int)$this->_cache->load(self::CACHE_KEY_LAST_SCHEDULE_GENERATE_AT . $groupId); - $rawSchedulePeriod = (int)$this->_scopeConfig->getValue( + $lastRun = (int)$this->cache->load(self::CACHE_KEY_LAST_SCHEDULE_GENERATE_AT . $groupId); + $rawSchedulePeriod = (int)$this->scopeConfig->getValue( 'system/cron/' . $groupId . '/' . self::XML_PATH_SCHEDULE_GENERATE_EVERY, \Magento\Store\Model\ScopeInterface::SCOPE_STORE ); @@ -326,7 +326,7 @@ protected function _generate($groupId) return $this; } - $schedules = $this->_getPendingSchedules(); + $schedules = $this->getPendingSchedules(); $exists = []; /** @var Schedule $schedule */ foreach ($schedules as $schedule) { @@ -336,13 +336,13 @@ protected function _generate($groupId) /** * generate global crontab jobs */ - $jobs = $this->_config->getJobs(); - $this->_generateJobs($jobs[$groupId], $exists, $groupId); + $jobs = $this->config->getJobs(); + $this->generateJobs($jobs[$groupId], $exists, $groupId); /** * save time schedules generation was ran with no expiration */ - $this->_cache->save( + $this->cache->save( $this->timezone->scopeTimeStamp(), self::CACHE_KEY_LAST_SCHEDULE_GENERATE_AT . $groupId, ['crontab'], @@ -360,7 +360,7 @@ protected function _generate($groupId) * @param string $groupId * @return $this */ - protected function _generateJobs($jobs, $exists, $groupId) + protected function generateJobs($jobs, $exists, $groupId) { foreach ($jobs as $jobCode => $jobConfig) { $cronExpression = null; @@ -390,11 +390,11 @@ protected function _generateJobs($jobs, $exists, $groupId) * @param string $groupId * @return $this */ - protected function _cleanup($groupId) + protected function cleanup($groupId) { // check if history cleanup is needed - $lastCleanup = (int)$this->_cache->load(self::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . $groupId); - $historyCleanUp = (int)$this->_scopeConfig->getValue( + $lastCleanup = (int)$this->cache->load(self::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . $groupId); + $historyCleanUp = (int)$this->scopeConfig->getValue( 'system/cron/' . $groupId . '/' . self::XML_PATH_HISTORY_CLEANUP_EVERY, \Magento\Store\Model\ScopeInterface::SCOPE_STORE ); @@ -403,7 +403,7 @@ protected function _cleanup($groupId) } // check how long the record should stay unprocessed before marked as MISSED - $scheduleLifetime = (int)$this->_scopeConfig->getValue( + $scheduleLifetime = (int)$this->scopeConfig->getValue( 'system/cron/' . $groupId . '/' . self::XML_PATH_SCHEDULE_LIFETIME, \Magento\Store\Model\ScopeInterface::SCOPE_STORE ); @@ -412,16 +412,16 @@ protected function _cleanup($groupId) /** * @var \Magento\Cron\Model\ResourceModel\Schedule\Collection $history */ - $history = $this->_scheduleFactory->create()->getCollection()->addFieldToFilter( + $history = $this->scheduleFactory->create()->getCollection()->addFieldToFilter( 'status', ['in' => [Schedule::STATUS_SUCCESS, Schedule::STATUS_MISSED, Schedule::STATUS_ERROR]] )->load(); - $historySuccess = (int)$this->_scopeConfig->getValue( + $historySuccess = (int)$this->scopeConfig->getValue( 'system/cron/' . $groupId . '/' . self::XML_PATH_HISTORY_SUCCESS, \Magento\Store\Model\ScopeInterface::SCOPE_STORE ); - $historyFailure = (int)$this->_scopeConfig->getValue( + $historyFailure = (int)$this->scopeConfig->getValue( 'system/cron/' . $groupId . '/' . self::XML_PATH_HISTORY_FAILURE, \Magento\Store\Model\ScopeInterface::SCOPE_STORE ); @@ -442,7 +442,7 @@ protected function _cleanup($groupId) } // save time history cleanup was ran with no expiration - $this->_cache->save( + $this->cache->save( $this->timezone->scopeTimeStamp(), self::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . $groupId, ['crontab'], @@ -458,7 +458,7 @@ protected function _cleanup($groupId) */ protected function getConfigSchedule($jobConfig) { - $cronExpr = $this->_scopeConfig->getValue( + $cronExpr = $this->scopeConfig->getValue( $jobConfig['config_path'], \Magento\Store\Model\ScopeInterface::SCOPE_STORE ); @@ -499,7 +499,7 @@ protected function saveSchedule($jobCode, $cronExpression, $timeInterval, $exist */ protected function generateSchedule($jobCode, $cronExpression, $time) { - $schedule = $this->_scheduleFactory->create() + $schedule = $this->scheduleFactory->create() ->setCronExpr($cronExpression) ->setJobCode($jobCode) ->setStatus(Schedule::STATUS_PENDING) @@ -515,7 +515,7 @@ protected function generateSchedule($jobCode, $cronExpression, $time) */ protected function getScheduleTimeInterval($groupId) { - $scheduleAheadFor = (int)$this->_scopeConfig->getValue( + $scheduleAheadFor = (int)$this->scopeConfig->getValue( 'system/cron/' . $groupId . '/' . self::XML_PATH_SCHEDULE_AHEAD_FOR, \Magento\Store\Model\ScopeInterface::SCOPE_STORE ); From 8ffa1914b416de876128f54b76f42374d8f8b642 Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Wed, 14 Jun 2017 15:27:34 +0600 Subject: [PATCH 02/15] Change the order or run, schedule, cleanup to cleanup, schedule, run This is in preparation of listening for changes in the configuration to check if a scheduled cron job is still enabled. When you disable a cron job in te config and flush the config cache, the cron observer should remove any scheduled cron jobs that are pending. If the cron observer runs the jobs before cleaning up, cron jobs that have been disabled will still run 1 time before they are removed from the schedule after disabling them. The extra advantage of this setup is that cron jobs will run directly after they have been scheduled, in the same run. So you don't have to run the cron job twice to see jobs executing. One for scheduling and one for running. If there is a perfectly good reason not to generate the schedule and clean jobs before running them I would very much like to know. --- app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index 3946b0f224746..26e4619a879b1 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -176,6 +176,8 @@ public function execute(\Magento\Framework\Event\Observer $observer) $phpPath = $this->phpExecutableFinder->find() ?: 'php'; foreach ($jobGroupsRoot as $groupId => $jobsRoot) { + $this->cleanup($groupId); + $this->generate($groupId); if ($this->request->getParam('group') !== null && $this->request->getParam('group') !== '\'' . ($groupId) . '\'' && $this->request->getParam('group') !== $groupId) { @@ -232,9 +234,6 @@ public function execute(\Magento\Framework\Event\Observer $observer) } $schedule->save(); } - - $this->generate($groupId); - $this->cleanup($groupId); } } From 5053fa029d9cd515c07aa511b63a02d550b301eb Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Wed, 14 Jun 2017 15:33:54 +0600 Subject: [PATCH 03/15] Make cron observer methods public. As I heard from @ishakhsuvarov Magento does not encourage usage of `protected` methods, because they do not encourage inheritance-based API. So composition in favor of inheritance. It's easier to maintain and reuse. I myself have already encountered scenarios where I wanted to use methods in this observer elsewhere in custom code, but I had to write duplicate code because these methods were `protected`. --- .../Cron/Observer/ProcessCronQueueObserver.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index 26e4619a879b1..29d9ba9f88512 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -248,7 +248,7 @@ public function execute(\Magento\Framework\Event\Observer $observer) * @return void * @throws \Exception */ - protected function runJob($scheduledTime, $currentTime, $jobConfig, $schedule, $groupId) + public function runJob($scheduledTime, $currentTime, $jobConfig, $schedule, $groupId) { $scheduleLifetime = (int)$this->scopeConfig->getValue( 'system/cron/' . $groupId . '/' . self::XML_PATH_SCHEDULE_LIFETIME, @@ -293,7 +293,7 @@ protected function runJob($scheduledTime, $currentTime, $jobConfig, $schedule, $ * * @return \Magento\Cron\Model\ResourceModel\Schedule\Collection */ - protected function getPendingSchedules() + public function getPendingSchedules() { if (!$this->pendingSchedules) { $this->pendingSchedules = $this->scheduleFactory->create()->getCollection()->addFieldToFilter( @@ -310,7 +310,7 @@ protected function getPendingSchedules() * @param string $groupId * @return $this */ - protected function generate($groupId) + public function generate($groupId) { /** * check if schedule generation is needed @@ -359,7 +359,7 @@ protected function generate($groupId) * @param string $groupId * @return $this */ - protected function generateJobs($jobs, $exists, $groupId) + public function generateJobs($jobs, $exists, $groupId) { foreach ($jobs as $jobCode => $jobConfig) { $cronExpression = null; @@ -389,7 +389,7 @@ protected function generateJobs($jobs, $exists, $groupId) * @param string $groupId * @return $this */ - protected function cleanup($groupId) + public function cleanup($groupId) { // check if history cleanup is needed $lastCleanup = (int)$this->cache->load(self::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . $groupId); @@ -455,7 +455,7 @@ protected function cleanup($groupId) * @param array $jobConfig * @return mixed */ - protected function getConfigSchedule($jobConfig) + public function getConfigSchedule($jobConfig) { $cronExpr = $this->scopeConfig->getValue( $jobConfig['config_path'], @@ -472,7 +472,7 @@ protected function getConfigSchedule($jobConfig) * @param array $exists * @return void */ - protected function saveSchedule($jobCode, $cronExpression, $timeInterval, $exists) + public function saveSchedule($jobCode, $cronExpression, $timeInterval, $exists) { $currentTime = $this->timezone->scopeTimeStamp(); $timeAhead = $currentTime + $timeInterval; @@ -496,7 +496,7 @@ protected function saveSchedule($jobCode, $cronExpression, $timeInterval, $exist * @param int $time * @return Schedule */ - protected function generateSchedule($jobCode, $cronExpression, $time) + public function generateSchedule($jobCode, $cronExpression, $time) { $schedule = $this->scheduleFactory->create() ->setCronExpr($cronExpression) @@ -512,7 +512,7 @@ protected function generateSchedule($jobCode, $cronExpression, $time) * @param string $groupId * @return int */ - protected function getScheduleTimeInterval($groupId) + public function getScheduleTimeInterval($groupId) { $scheduleAheadFor = (int)$this->scopeConfig->getValue( 'system/cron/' . $groupId . '/' . self::XML_PATH_SCHEDULE_AHEAD_FOR, From 8f24e0f951e5c65b3c8a4ff0cc54237b41e0689e Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Wed, 14 Jun 2017 16:06:30 +0600 Subject: [PATCH 04/15] clean up pending jobs that have been disabled in the config --- .../Observer/ProcessCronQueueObserver.php | 53 ++++++++++++++----- 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index 29d9ba9f88512..45588f85dfeaa 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -362,17 +362,7 @@ public function generate($groupId) public function generateJobs($jobs, $exists, $groupId) { foreach ($jobs as $jobCode => $jobConfig) { - $cronExpression = null; - if (isset($jobConfig['config_path'])) { - $cronExpression = $this->getConfigSchedule($jobConfig) ?: null; - } - - if (!$cronExpression) { - if (isset($jobConfig['schedule'])) { - $cronExpression = $jobConfig['schedule']; - } - } - + $cronExpression = $this->getCronExpression($jobConfig); if (!$cronExpression) { continue; } @@ -384,13 +374,15 @@ public function generateJobs($jobs, $exists, $groupId) } /** - * Clean existed jobs + * Clean expired jobs * * @param string $groupId * @return $this */ public function cleanup($groupId) { + $this->cleanupDisabledJobs($groupId); + // check if history cleanup is needed $lastCleanup = (int)$this->cache->load(self::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . $groupId); $historyCleanUp = (int)$this->scopeConfig->getValue( @@ -522,4 +514,41 @@ public function getScheduleTimeInterval($groupId) return $scheduleAheadFor; } + + /** + * @param $groupId + */ + public function cleanupDisabledJobs($groupId) + { + $jobs = $this->config->getJobs(); + foreach ($jobs[$groupId] as $jobCode => $jobConfig) { + if (!$this->getCronExpression($jobConfig)) { + /** @var \Magento\Cron\Model\ResourceModel\Schedule $scheduleResource */ + $scheduleResource = $this->scheduleFactory->create()->getResource(); + $scheduleResource->getConnection()->delete($scheduleResource->getMainTable(), [ + 'status=?' => Schedule::STATUS_PENDING, + 'job_code=?' => $jobCode, + ]); + } + } + } + + /** + * @param $jobConfig + * @return null|string + */ + public function getCronExpression($jobConfig) + { + $cronExpression = null; + if (isset($jobConfig['config_path'])) { + $cronExpression = $this->getConfigSchedule($jobConfig) ?: null; + } + + if (!$cronExpression) { + if (isset($jobConfig['schedule'])) { + $cronExpression = $jobConfig['schedule']; + } + } + return $cronExpression; + } } From 48f8ca437ad60323008c230a8588479b46c219ec Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Wed, 14 Jun 2017 17:15:08 +0600 Subject: [PATCH 05/15] The variable name $ts is more clear if it is named $timestamp --- app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index 45588f85dfeaa..fb2804c901f2b 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -469,8 +469,8 @@ public function saveSchedule($jobCode, $cronExpression, $timeInterval, $exists) $currentTime = $this->timezone->scopeTimeStamp(); $timeAhead = $currentTime + $timeInterval; for ($time = $currentTime; $time < $timeAhead; $time += self::SECONDS_IN_MINUTE) { - $ts = strftime('%Y-%m-%d %H:%M:00', $time); - if (!empty($exists[$jobCode . '/' . $ts])) { + $timestamp = strftime('%Y-%m-%d %H:%M:00', $time); + if (!empty($exists[$jobCode . '/' . $timestamp])) { // already scheduled continue; } From 1cf8ba5bb109ec81be6aa6ef290cc73397017a20 Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Wed, 14 Jun 2017 18:54:54 +0600 Subject: [PATCH 06/15] Remove scheduled jobs that do not match their cron expression When you change a cron expression in the config, you do not want jobs that were scheduled ahead to be executed on the old config cron expression. The cleanup makes the config change take immediate effect. --- .../Observer/ProcessCronQueueObserver.php | 57 ++++++++++++++++--- 1 file changed, 48 insertions(+), 9 deletions(-) diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index fb2804c901f2b..b608303a01590 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -109,12 +109,17 @@ class ProcessCronQueueObserver implements ObserverInterface /** * @var \Psr\Log\LoggerInterface */ - private $logger; + protected $logger; /** * @var \Magento\Framework\App\State */ - private $state; + protected $state; + + /** + * @var array + */ + protected $invalid = []; /** * @param \Magento\Framework\ObjectManagerInterface $objectManager @@ -180,7 +185,8 @@ public function execute(\Magento\Framework\Event\Observer $observer) $this->generate($groupId); if ($this->request->getParam('group') !== null && $this->request->getParam('group') !== '\'' . ($groupId) . '\'' - && $this->request->getParam('group') !== $groupId) { + && $this->request->getParam('group') !== $groupId + ) { continue; } if (($this->request->getParam(self::STANDALONE_PROCESS_STARTED) !== '1') && ( @@ -188,7 +194,8 @@ public function execute(\Magento\Framework\Event\Observer $observer) 'system/cron/' . $groupId . '/use_separate_process', \Magento\Store\Model\ScopeInterface::SCOPE_STORE ) == 1 - )) { + ) + ) { $this->shell->execute( $phpPath . ' %s cron:run --group=' . $groupId . ' --' . Cli::INPUT_KEY_BOOTSTRAP . '=' . self::STANDALONE_PROCESS_STARTED . '=1', @@ -336,7 +343,9 @@ public function generate($groupId) * generate global crontab jobs */ $jobs = $this->config->getJobs(); + $this->invalid = []; $this->generateJobs($jobs[$groupId], $exists, $groupId); + $this->cleanupScheduleMismatches(); /** * save time schedules generation was ran with no expiration @@ -469,13 +478,20 @@ public function saveSchedule($jobCode, $cronExpression, $timeInterval, $exists) $currentTime = $this->timezone->scopeTimeStamp(); $timeAhead = $currentTime + $timeInterval; for ($time = $currentTime; $time < $timeAhead; $time += self::SECONDS_IN_MINUTE) { - $timestamp = strftime('%Y-%m-%d %H:%M:00', $time); - if (!empty($exists[$jobCode . '/' . $timestamp])) { - // already scheduled + $scheduledAt = strftime('%Y-%m-%d %H:%M:00', $time); + $alreadyScheduled = !empty($exists[$jobCode . '/' . $scheduledAt]); + $schedule = $this->generateSchedule($jobCode, $cronExpression, $time); + $valid = $schedule->trySchedule(); + if (!$valid) { + if ($alreadyScheduled) { + if (!isset($this->invalid[$jobCode])) { + $this->invalid[$jobCode] = []; + } + $this->invalid[$jobCode][] = $scheduledAt; + } continue; } - $schedule = $this->generateSchedule($jobCode, $cronExpression, $time); - if ($schedule->trySchedule()) { + if (!$alreadyScheduled) { // time matches cron expression $schedule->save(); } @@ -516,6 +532,9 @@ public function getScheduleTimeInterval($groupId) } /** + * Clean up scheduled jobs that are disabled in the configuration + * This can happen when you turn off a cron job in the config and flush the cache + * * @param $groupId */ public function cleanupDisabledJobs($groupId) @@ -551,4 +570,24 @@ public function getCronExpression($jobConfig) } return $cronExpression; } + + /** + * Clean up scheduled jobs that do not match their cron expression anymore + * This can happen when you change the cron expression and flush the cache + * + * @return $this + */ + public function cleanupScheduleMismatches() + { + foreach ($this->invalid as $jobCode => $scheduledAtList) { + /** @var \Magento\Cron\Model\ResourceModel\Schedule $scheduleResource */ + $scheduleResource = $this->scheduleFactory->create()->getResource(); + $scheduleResource->getConnection()->delete($scheduleResource->getMainTable(), [ + 'status=?' => Schedule::STATUS_PENDING, + 'job_code=?' => $jobCode, + 'scheduled_at in (?)' => $scheduledAtList, + ]); + } + return $this; + } } From 0518a6c30f599785989c1b69125f764012540f62 Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Thu, 15 Jun 2017 09:11:18 +0600 Subject: [PATCH 07/15] Clean up underscores before making other changes --- .../Observer/ProcessCronQueueObserverTest.php | 278 +++++++++--------- 1 file changed, 140 insertions(+), 138 deletions(-) diff --git a/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php b/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php index fadd9e52a7fe1..30566270dedb3 100644 --- a/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php +++ b/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php @@ -18,50 +18,52 @@ class ProcessCronQueueObserverTest extends \PHPUnit_Framework_TestCase /** * @var ProcessCronQueueObserver */ - protected $_observer; + protected $cronQueueObserver; /** * @var \Magento\Framework\App\ObjectManager|\PHPUnit_Framework_MockObject_MockObject */ - protected $_objectManager; + protected $objectManager; /** * @var \PHPUnit_Framework_MockObject_MockObject */ - protected $_cache; + protected $cache; /** * @var \Magento\Cron\Model\Config|\PHPUnit_Framework_MockObject_MockObject */ - protected $_config; + protected $config; /** * @var \Magento\Cron\Model\ScheduleFactory */ - protected $_scheduleFactory; + protected $scheduleFactory; /** * @var \Magento\Framework\App\Config\ScopeConfigInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $_scopeConfig; + protected $scopeConfig; /** * @var \Magento\Framework\App\Console\Request|\PHPUnit_Framework_MockObject_MockObject */ - protected $_request; + protected $request; /** * @var \Magento\Framework\ShellInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $_shell; + protected $shell; - /** @var \Magento\Cron\Model\ResourceModel\Schedule\Collection|\PHPUnit_Framework_MockObject_MockObject */ - protected $_collection; + /** + * @var \Magento\Cron\Model\ResourceModel\Schedule\Collection|\PHPUnit_Framework_MockObject_MockObject + */ + protected $collection; /** * @var \Magento\Cron\Model\Groups\Config\Data */ - protected $_cronGroupConfig; + protected $cronGroupConfig; /** * @var \Magento\Framework\Stdlib\DateTime\TimezoneInterface @@ -88,32 +90,32 @@ class ProcessCronQueueObserverTest extends \PHPUnit_Framework_TestCase */ protected function setUp() { - $this->_objectManager = $this->getMockBuilder( + $this->objectManager = $this->getMockBuilder( \Magento\Framework\App\ObjectManager::class )->disableOriginalConstructor()->getMock(); - $this->_cache = $this->getMock(\Magento\Framework\App\CacheInterface::class); - $this->_config = $this->getMockBuilder( + $this->cache = $this->getMock(\Magento\Framework\App\CacheInterface::class); + $this->config = $this->getMockBuilder( \Magento\Cron\Model\Config::class )->disableOriginalConstructor()->getMock(); - $this->_scopeConfig = $this->getMockBuilder( + $this->scopeConfig = $this->getMockBuilder( \Magento\Framework\App\Config\ScopeConfigInterface::class )->disableOriginalConstructor()->getMock(); - $this->_collection = $this->getMockBuilder( + $this->collection = $this->getMockBuilder( \Magento\Cron\Model\ResourceModel\Schedule\Collection::class )->setMethods( ['addFieldToFilter', 'load', '__wakeup'] )->disableOriginalConstructor()->getMock(); - $this->_collection->expects($this->any())->method('addFieldToFilter')->will($this->returnSelf()); - $this->_collection->expects($this->any())->method('load')->will($this->returnSelf()); - $this->_scheduleFactory = $this->getMockBuilder( + $this->collection->expects($this->any())->method('addFieldToFilter')->will($this->returnSelf()); + $this->collection->expects($this->any())->method('load')->will($this->returnSelf()); + $this->scheduleFactory = $this->getMockBuilder( \Magento\Cron\Model\ScheduleFactory::class )->setMethods( ['create'] )->disableOriginalConstructor()->getMock(); - $this->_request = $this->getMockBuilder( + $this->request = $this->getMockBuilder( \Magento\Framework\App\Console\Request::class )->disableOriginalConstructor()->getMock(); - $this->_shell = $this->getMockBuilder( + $this->shell = $this->getMockBuilder( \Magento\Framework\ShellInterface::class )->disableOriginalConstructor()->setMethods( ['execute'] @@ -140,14 +142,14 @@ protected function setUp() ); $phpExecutableFinderFactory->expects($this->any())->method('create')->willReturn($phpExecutableFinder); - $this->_observer = new ProcessCronQueueObserver( - $this->_objectManager, - $this->_scheduleFactory, - $this->_cache, - $this->_config, - $this->_scopeConfig, - $this->_request, - $this->_shell, + $this->cronQueueObserver = new ProcessCronQueueObserver( + $this->objectManager, + $this->scheduleFactory, + $this->cache, + $this->config, + $this->scopeConfig, + $this->request, + $this->shell, $this->timezone, $phpExecutableFinderFactory, $this->loggerMock, @@ -161,18 +163,18 @@ protected function setUp() public function testDispatchNoPendingJobs() { $lastRun = time() + 10000000; - $this->_cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); - $this->_scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(0)); + $this->cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); + $this->scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(0)); - $this->_config->expects($this->once())->method('getJobs')->will($this->returnValue([])); + $this->config->expects($this->once())->method('getJobs')->will($this->returnValue([])); $scheduleMock = $this->getMockBuilder( \Magento\Cron\Model\Schedule::class )->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->_collection)); - $this->_scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); + $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $this->scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); - $this->_observer->execute($this->observer); + $this->cronQueueObserver->execute($this->observer); } /** @@ -181,10 +183,10 @@ public function testDispatchNoPendingJobs() public function testDispatchNoJobConfig() { $lastRun = time() + 10000000; - $this->_cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); - $this->_scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(0)); + $this->cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); + $this->scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(0)); - $this->_config->expects( + $this->config->expects( $this->once() )->method( 'getJobs' @@ -195,17 +197,17 @@ public function testDispatchNoJobConfig() $schedule = $this->getMock(\Magento\Cron\Model\Schedule::class, ['getJobCode', '__wakeup'], [], '', false); $schedule->expects($this->once())->method('getJobCode')->will($this->returnValue('not_existed_job_code')); - $this->_collection->addItem($schedule); + $this->collection->addItem($schedule); - $this->_config->expects($this->once())->method('getJobs')->will($this->returnValue([])); + $this->config->expects($this->once())->method('getJobs')->will($this->returnValue([])); $scheduleMock = $this->getMockBuilder( \Magento\Cron\Model\Schedule::class )->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->_collection)); - $this->_scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); + $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $this->scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); - $this->_observer->execute($this->observer); + $this->cronQueueObserver->execute($this->observer); } /** @@ -214,9 +216,9 @@ public function testDispatchNoJobConfig() public function testDispatchCanNotLock() { $lastRun = time() + 10000000; - $this->_cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); - $this->_scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(0)); - $this->_request->expects($this->any())->method('getParam')->will($this->returnValue('test_group')); + $this->cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); + $this->scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(0)); + $this->request->expects($this->any())->method('getParam')->will($this->returnValue('test_group')); $schedule = $this->getMockBuilder( \Magento\Cron\Model\Schedule::class )->setMethods( @@ -227,9 +229,9 @@ public function testDispatchCanNotLock() $schedule->expects($this->once())->method('tryLockJob')->will($this->returnValue(false)); $abstractModel = $this->getMock(\Magento\Framework\Model\AbstractModel::class, [], [], '', false); $schedule->expects($this->any())->method('save')->will($this->returnValue($abstractModel)); - $this->_collection->addItem($schedule); + $this->collection->addItem($schedule); - $this->_config->expects( + $this->config->expects( $this->once() )->method( 'getJobs' @@ -240,10 +242,10 @@ public function testDispatchCanNotLock() $scheduleMock = $this->getMockBuilder( \Magento\Cron\Model\Schedule::class )->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->_collection)); - $this->_scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); + $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $this->scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); - $this->_observer->execute($this->observer); + $this->cronQueueObserver->execute($this->observer); } /** @@ -257,9 +259,9 @@ public function testDispatchExceptionTooLate() $exception = $exceptionMessage . ' Schedule Id: ' . $scheduleId . ' Job Code: ' . $jobCode; $lastRun = time() + 10000000; - $this->_cache->expects($this->any())->method('load')->willReturn($lastRun); - $this->_scopeConfig->expects($this->any())->method('getValue')->willReturn(0); - $this->_request->expects($this->any())->method('getParam')->willReturn('test_group'); + $this->cache->expects($this->any())->method('load')->willReturn($lastRun); + $this->scopeConfig->expects($this->any())->method('getValue')->willReturn(0); + $this->request->expects($this->any())->method('getParam')->willReturn('test_group'); $schedule = $this->getMockBuilder( \Magento\Cron\Model\Schedule::class )->setMethods( @@ -296,9 +298,9 @@ public function testDispatchExceptionTooLate() $this->loggerMock->expects($this->once())->method('info')->with($exception); - $this->_collection->addItem($schedule); + $this->collection->addItem($schedule); - $this->_config->expects( + $this->config->expects( $this->once() )->method( 'getJobs' @@ -308,10 +310,10 @@ public function testDispatchExceptionTooLate() $scheduleMock = $this->getMockBuilder(\Magento\Cron\Model\Schedule::class) ->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->willReturn($this->_collection); - $this->_scheduleFactory->expects($this->once())->method('create')->willReturn($scheduleMock); + $scheduleMock->expects($this->any())->method('getCollection')->willReturn($this->collection); + $this->scheduleFactory->expects($this->once())->method('create')->willReturn($scheduleMock); - $this->_observer->execute($this->observer); + $this->cronQueueObserver->execute($this->observer); } /** @@ -342,27 +344,27 @@ public function testDispatchExceptionNoCallback() $schedule->expects($this->once())->method('setMessages')->with($this->equalTo($exceptionMessage)); $schedule->expects($this->any())->method('getStatus')->willReturn(Schedule::STATUS_ERROR); $schedule->expects($this->once())->method('save'); - $this->_request->expects($this->any())->method('getParam')->will($this->returnValue('test_group')); - $this->_collection->addItem($schedule); + $this->request->expects($this->any())->method('getParam')->will($this->returnValue('test_group')); + $this->collection->addItem($schedule); $this->loggerMock->expects($this->once())->method('critical')->with($exception); $jobConfig = ['test_group' => ['test_job1' => ['instance' => 'Some_Class']]]; - $this->_config->expects($this->once())->method('getJobs')->will($this->returnValue($jobConfig)); + $this->config->expects($this->once())->method('getJobs')->will($this->returnValue($jobConfig)); $lastRun = time() + 10000000; - $this->_cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); + $this->cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); - $this->_scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(strtotime('+1 day'))); + $this->scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(strtotime('+1 day'))); $scheduleMock = $this->getMockBuilder( \Magento\Cron\Model\Schedule::class )->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->_collection)); - $this->_scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); + $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $this->scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); - $this->_observer->execute($this->observer); + $this->cronQueueObserver->execute($this->observer); } /** @@ -389,7 +391,7 @@ public function testDispatchExceptionInCallback( ], ]; - $this->_request->expects($this->any())->method('getParam')->will($this->returnValue('test_group')); + $this->request->expects($this->any())->method('getParam')->will($this->returnValue('test_group')); $schedule = $this->getMockBuilder( \Magento\Cron\Model\Schedule::class )->setMethods( @@ -408,26 +410,26 @@ public function testDispatchExceptionInCallback( $this->loggerMock->expects($this->once())->method('critical')->with($exception); - $this->_collection->addItem($schedule); + $this->collection->addItem($schedule); - $this->_config->expects($this->once())->method('getJobs')->will($this->returnValue($jobConfig)); + $this->config->expects($this->once())->method('getJobs')->will($this->returnValue($jobConfig)); $lastRun = time() + 10000000; - $this->_cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); - $this->_scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(strtotime('+1 day'))); + $this->cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); + $this->scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(strtotime('+1 day'))); $scheduleMock = $this->getMockBuilder( \Magento\Cron\Model\Schedule::class )->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->_collection)); - $this->_scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); - $this->_objectManager + $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $this->scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); + $this->objectManager ->expects($this->once()) ->method('create') ->with($this->equalTo($cronJobType)) ->will($this->returnValue($cronJobObject)); - $this->_observer->execute($this->observer); + $this->cronQueueObserver->execute($this->observer); } /** @@ -461,7 +463,7 @@ public function testDispatchRunJob() $jobConfig = [ 'test_group' => ['test_job1' => ['instance' => 'CronJob', 'method' => 'execute']], ]; - $this->_request->expects($this->any())->method('getParam')->will($this->returnValue('test_group')); + $this->request->expects($this->any())->method('getParam')->will($this->returnValue('test_group')); $scheduleMethods = [ 'getJobCode', @@ -501,24 +503,24 @@ public function testDispatchRunJob() $schedule->expects($this->at(8))->method('save'); - $this->_collection->addItem($schedule); + $this->collection->addItem($schedule); - $this->_config->expects($this->once())->method('getJobs')->will($this->returnValue($jobConfig)); + $this->config->expects($this->once())->method('getJobs')->will($this->returnValue($jobConfig)); $lastRun = time() + 10000000; - $this->_cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); - $this->_scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(strtotime('+1 day'))); + $this->cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); + $this->scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(strtotime('+1 day'))); $scheduleMock = $this->getMockBuilder( \Magento\Cron\Model\Schedule::class )->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->_collection)); - $this->_scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); + $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $this->scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); $testCronJob = $this->getMockBuilder('CronJob')->setMethods(['execute'])->getMock(); $testCronJob->expects($this->atLeastOnce())->method('execute')->with($schedule); - $this->_objectManager->expects( + $this->objectManager->expects( $this->once() )->method( 'create' @@ -528,11 +530,11 @@ public function testDispatchRunJob() $this->returnValue($testCronJob) ); - $this->_observer->execute($this->observer); + $this->cronQueueObserver->execute($this->observer); } /** - * Testing _generate(), iterate over saved cron jobs + * Testing generate(), iterate over saved cron jobs */ public function testDispatchNotGenerate() { @@ -540,16 +542,16 @@ public function testDispatchNotGenerate() 'test_group' => ['test_job1' => ['instance' => 'CronJob', 'method' => 'execute']], ]; - $this->_config->expects($this->at(0))->method('getJobs')->will($this->returnValue($jobConfig)); - $this->_config->expects( + $this->config->expects($this->at(0))->method('getJobs')->will($this->returnValue($jobConfig)); + $this->config->expects( $this->at(1) )->method( 'getJobs' )->will( $this->returnValue(['test_group' => []]) ); - $this->_request->expects($this->any())->method('getParam')->will($this->returnValue('test_group')); - $this->_cache->expects( + $this->request->expects($this->any())->method('getParam')->will($this->returnValue('test_group')); + $this->cache->expects( $this->at(0) )->method( 'load' @@ -558,7 +560,7 @@ public function testDispatchNotGenerate() )->will( $this->returnValue(time() - 10000000) ); - $this->_cache->expects( + $this->cache->expects( $this->at(2) )->method( 'load' @@ -568,7 +570,7 @@ public function testDispatchNotGenerate() $this->returnValue(time() + 10000000) ); - $this->_scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(0)); + $this->scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(0)); $schedule = $this->getMockBuilder( \Magento\Cron\Model\Schedule::class @@ -578,24 +580,24 @@ public function testDispatchNotGenerate() $schedule->expects($this->any())->method('getJobCode')->will($this->returnValue('job_code1')); $schedule->expects($this->once())->method('getScheduledAt')->will($this->returnValue('* * * * *')); - $this->_collection->addItem(new \Magento\Framework\DataObject()); - $this->_collection->addItem($schedule); + $this->collection->addItem(new \Magento\Framework\DataObject()); + $this->collection->addItem($schedule); - $this->_cache->expects($this->any())->method('save'); + $this->cache->expects($this->any())->method('save'); $scheduleMock = $this->getMockBuilder( \Magento\Cron\Model\Schedule::class )->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->_collection)); - $this->_scheduleFactory->expects($this->any())->method('create')->will($this->returnValue($scheduleMock)); + $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $this->scheduleFactory->expects($this->any())->method('create')->will($this->returnValue($scheduleMock)); - $this->_scheduleFactory->expects($this->any())->method('create')->will($this->returnValue($schedule)); + $this->scheduleFactory->expects($this->any())->method('create')->will($this->returnValue($schedule)); - $this->_observer->execute($this->observer); + $this->cronQueueObserver->execute($this->observer); } /** - * Testing _generate(), iterate over saved cron jobs and generate jobs + * Testing generate(), iterate over saved cron jobs and generate jobs */ public function testDispatchGenerate() { @@ -615,17 +617,17 @@ public function testDispatchGenerate() 'job3' => ['schedule' => '* * * * *'], ], ]; - $this->_config->expects($this->at(0))->method('getJobs')->willReturn($jobConfig); - $this->_config->expects($this->at(1))->method('getJobs')->willReturn($jobs); - $this->_request->expects($this->any())->method('getParam')->willReturn('default'); - $this->_cache->expects( + $this->config->expects($this->at(0))->method('getJobs')->willReturn($jobConfig); + $this->config->expects($this->at(1))->method('getJobs')->willReturn($jobs); + $this->request->expects($this->any())->method('getParam')->willReturn('default'); + $this->cache->expects( $this->at(0) )->method( 'load' )->with( $this->equalTo(ProcessCronQueueObserver::CACHE_KEY_LAST_SCHEDULE_GENERATE_AT . 'default') )->willReturn(time() - 10000000); - $this->_cache->expects( + $this->cache->expects( $this->at(2) )->method( 'load' @@ -633,7 +635,7 @@ public function testDispatchGenerate() $this->equalTo(ProcessCronQueueObserver::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . 'default') )->willReturn(time() + 10000000); - $this->_scopeConfig->expects($this->any())->method('getValue')->willReturnMap( + $this->scopeConfig->expects($this->any())->method('getValue')->willReturnMap( [ [ 'system/cron/default/schedule_generate_every', @@ -659,17 +661,17 @@ public function testDispatchGenerate() $schedule->expects($this->once())->method('getScheduledAt')->willReturn('* * * * *'); $schedule->expects($this->any())->method('unsScheduleId')->willReturnSelf(); $schedule->expects($this->any())->method('trySchedule')->willReturnSelf(); - $schedule->expects($this->any())->method('getCollection')->willReturn($this->_collection); + $schedule->expects($this->any())->method('getCollection')->willReturn($this->collection); $schedule->expects($this->atLeastOnce())->method('save')->willReturnSelf(); - $this->_collection->addItem(new \Magento\Framework\DataObject()); - $this->_collection->addItem($schedule); + $this->collection->addItem(new \Magento\Framework\DataObject()); + $this->collection->addItem($schedule); - $this->_cache->expects($this->any())->method('save'); + $this->cache->expects($this->any())->method('save'); - $this->_scheduleFactory->expects($this->any())->method('create')->willReturn($schedule); + $this->scheduleFactory->expects($this->any())->method('create')->willReturn($schedule); - $this->_observer->execute($this->observer); + $this->cronQueueObserver->execute($this->observer); } /** @@ -688,21 +690,21 @@ public function testDispatchCleanup() )->getMock(); $schedule->expects($this->any())->method('getExecutedAt')->will($this->returnValue('-1 day')); $schedule->expects($this->any())->method('getStatus')->will($this->returnValue('success')); - $this->_request->expects($this->any())->method('getParam')->will($this->returnValue('test_group')); - $this->_collection->addItem($schedule); + $this->request->expects($this->any())->method('getParam')->will($this->returnValue('test_group')); + $this->collection->addItem($schedule); - $this->_config->expects($this->once())->method('getJobs')->will($this->returnValue($jobConfig)); + $this->config->expects($this->once())->method('getJobs')->will($this->returnValue($jobConfig)); - $this->_cache->expects($this->at(0))->method('load')->will($this->returnValue(time() + 10000000)); - $this->_cache->expects($this->at(1))->method('load')->will($this->returnValue(time() - 10000000)); + $this->cache->expects($this->at(0))->method('load')->will($this->returnValue(time() + 10000000)); + $this->cache->expects($this->at(1))->method('load')->will($this->returnValue(time() - 10000000)); - $this->_scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(0)); + $this->scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(0)); $scheduleMock = $this->getMockBuilder( \Magento\Cron\Model\Schedule::class )->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->_collection)); - $this->_scheduleFactory->expects($this->at(0))->method('create')->will($this->returnValue($scheduleMock)); + $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $this->scheduleFactory->expects($this->at(0))->method('create')->will($this->returnValue($scheduleMock)); $collection = $this->getMockBuilder( \Magento\Cron\Model\ResourceModel\Schedule\Collection::class @@ -717,14 +719,14 @@ public function testDispatchCleanup() \Magento\Cron\Model\Schedule::class )->disableOriginalConstructor()->getMock(); $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($collection)); - $this->_scheduleFactory->expects($this->at(1))->method('create')->will($this->returnValue($scheduleMock)); + $this->scheduleFactory->expects($this->at(1))->method('create')->will($this->returnValue($scheduleMock)); - $this->_observer->execute($this->observer); + $this->cronQueueObserver->execute($this->observer); } public function testMissedJobsCleanedInTime() { - /* 1. Initialize dependencies of _generate() method which is called first */ + /* 1. Initialize dependencies of generate() method which is called first */ $jobConfig = [ 'test_group' => ['test_job1' => ['instance' => 'CronJob', 'method' => 'execute']], ]; @@ -754,39 +756,39 @@ public function testMissedJobsCleanedInTime() //we don't expect this job be deleted from the list $schedule2->expects($this->never())->method('delete'); - $this->_collection->addItem($schedule1); - $this->_config->expects($this->once())->method('getJobs')->will($this->returnValue($jobConfig)); + $this->collection->addItem($schedule1); + $this->config->expects($this->once())->method('getJobs')->will($this->returnValue($jobConfig)); //get configuration value CACHE_KEY_LAST_HISTORY_CLEANUP_AT in the "_generate()" - $this->_cache->expects($this->at(0))->method('load')->will($this->returnValue(time() + 10000000)); + $this->cache->expects($this->at(0))->method('load')->will($this->returnValue(time() + 10000000)); //get configuration value CACHE_KEY_LAST_HISTORY_CLEANUP_AT in the "_cleanup()" - $this->_cache->expects($this->at(1))->method('load')->will($this->returnValue(time() - 10000000)); + $this->cache->expects($this->at(1))->method('load')->will($this->returnValue(time() - 10000000)); - $this->_scopeConfig->expects($this->at(0))->method('getValue') + $this->scopeConfig->expects($this->at(0))->method('getValue') ->with($this->equalTo('system/cron/test_group/use_separate_process')) ->will($this->returnValue(0)); - $this->_scopeConfig->expects($this->at(1))->method('getValue') + $this->scopeConfig->expects($this->at(1))->method('getValue') ->with($this->equalTo('system/cron/test_group/schedule_generate_every')) ->will($this->returnValue(0)); - $this->_scopeConfig->expects($this->at(2))->method('getValue') + $this->scopeConfig->expects($this->at(2))->method('getValue') ->with($this->equalTo('system/cron/test_group/history_cleanup_every')) ->will($this->returnValue(0)); - $this->_scopeConfig->expects($this->at(3))->method('getValue') + $this->scopeConfig->expects($this->at(3))->method('getValue') ->with($this->equalTo('system/cron/test_group/schedule_lifetime')) ->will($this->returnValue(2*24*60)); - $this->_scopeConfig->expects($this->at(4))->method('getValue') + $this->scopeConfig->expects($this->at(4))->method('getValue') ->with($this->equalTo('system/cron/test_group/history_success_lifetime')) ->will($this->returnValue(0)); - $this->_scopeConfig->expects($this->at(5))->method('getValue') + $this->scopeConfig->expects($this->at(5))->method('getValue') ->with($this->equalTo('system/cron/test_group/history_failure_lifetime')) ->will($this->returnValue(0)); - /* 2. Initialize dependencies of _cleanup() method which is called second */ + /* 2. Initialize dependencies of cleanup() method which is called second */ $scheduleMock = $this->getMockBuilder( \Magento\Cron\Model\Schedule::class )->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->_collection)); - $this->_scheduleFactory->expects($this->at(0))->method('create')->will($this->returnValue($scheduleMock)); + $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $this->scheduleFactory->expects($this->at(0))->method('create')->will($this->returnValue($scheduleMock)); $collection = $this->getMockBuilder( \Magento\Cron\Model\ResourceModel\Schedule\Collection::class @@ -802,8 +804,8 @@ public function testMissedJobsCleanedInTime() \Magento\Cron\Model\Schedule::class )->disableOriginalConstructor()->getMock(); $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($collection)); - $this->_scheduleFactory->expects($this->at(1))->method('create')->will($this->returnValue($scheduleMock)); + $this->scheduleFactory->expects($this->at(1))->method('create')->will($this->returnValue($scheduleMock)); - $this->_observer->execute($this->observer); + $this->cronQueueObserver->execute($this->observer); } } From cd6d480f55b595b67c31db023f6699f51cdb9841 Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Thu, 15 Jun 2017 14:42:15 +0600 Subject: [PATCH 08/15] Make sure that all current unit tests are functioning again Some unit tests did not flag as failed even though the thing they were testing did not occur. I have rewritten these tests to be more responsive when things go wrong. There is however a big step to be made to make these tests more readable and more consistent. --- .../Observer/ProcessCronQueueObserver.php | 21 +- .../Observer/ProcessCronQueueObserverTest.php | 603 +++++++----------- 2 files changed, 242 insertions(+), 382 deletions(-) diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index b608303a01590..138a87563e136 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -121,6 +121,11 @@ class ProcessCronQueueObserver implements ObserverInterface */ protected $invalid = []; + /** + * @var array + */ + protected $jobs; + /** * @param \Magento\Framework\ObjectManagerInterface $objectManager * @param \Magento\Cron\Model\ScheduleFactory $scheduleFactory @@ -206,6 +211,7 @@ public function execute(\Magento\Framework\Event\Observer $observer) continue; } + /** @var \Magento\Cron\Model\Schedule $schedule */ foreach ($pendingJobs as $schedule) { $jobConfig = isset($jobsRoot[$schedule->getJobCode()]) ? $jobsRoot[$schedule->getJobCode()] : null; if (!$jobConfig) { @@ -342,7 +348,7 @@ public function generate($groupId) /** * generate global crontab jobs */ - $jobs = $this->config->getJobs(); + $jobs = $this->getJobs(); $this->invalid = []; $this->generateJobs($jobs[$groupId], $exists, $groupId); $this->cleanupScheduleMismatches(); @@ -539,7 +545,7 @@ public function getScheduleTimeInterval($groupId) */ public function cleanupDisabledJobs($groupId) { - $jobs = $this->config->getJobs(); + $jobs = $this->getJobs(); foreach ($jobs[$groupId] as $jobCode => $jobConfig) { if (!$this->getCronExpression($jobConfig)) { /** @var \Magento\Cron\Model\ResourceModel\Schedule $scheduleResource */ @@ -590,4 +596,15 @@ public function cleanupScheduleMismatches() } return $this; } + + /** + * @return array + */ + public function getJobs() + { + if (is_null($this->jobs)) { + $this->jobs = $this->config->getJobs(); + } + return $this->jobs; + } } diff --git a/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php b/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php index 30566270dedb3..9ec1a0f41f357 100644 --- a/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php +++ b/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php @@ -5,13 +5,17 @@ */ namespace Magento\Cron\Test\Unit\Observer; +use Magento\Cron\Model\ResourceModel\Schedule as ScheduleResource; use Magento\Cron\Model\Schedule; -use Magento\Cron\Observer\ProcessCronQueueObserver as ProcessCronQueueObserver; +use Magento\Cron\Observer\ProcessCronQueueObserver; use Magento\Framework\App\State; +use Magento\Framework\DB\Adapter\AdapterInterface; +use PHPUnit_Framework_MockObject_MockObject as Mock; /** * Class \Magento\Cron\Test\Unit\Model\ObserverTest * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + * @SuppressWarnings(PHPMD.TooManyFields) */ class ProcessCronQueueObserverTest extends \PHPUnit_Framework_TestCase { @@ -21,42 +25,42 @@ class ProcessCronQueueObserverTest extends \PHPUnit_Framework_TestCase protected $cronQueueObserver; /** - * @var \Magento\Framework\App\ObjectManager|\PHPUnit_Framework_MockObject_MockObject + * @var \Magento\Framework\App\ObjectManager | Mock */ protected $objectManager; /** - * @var \PHPUnit_Framework_MockObject_MockObject + * @var Mock */ protected $cache; /** - * @var \Magento\Cron\Model\Config|\PHPUnit_Framework_MockObject_MockObject + * @var \Magento\Cron\Model\Config | Mock */ protected $config; /** - * @var \Magento\Cron\Model\ScheduleFactory + * @var \Magento\Cron\Model\ScheduleFactory | Mock */ protected $scheduleFactory; /** - * @var \Magento\Framework\App\Config\ScopeConfigInterface|\PHPUnit_Framework_MockObject_MockObject + * @var \Magento\Framework\App\Config\ScopeConfigInterface | Mock */ protected $scopeConfig; /** - * @var \Magento\Framework\App\Console\Request|\PHPUnit_Framework_MockObject_MockObject + * @var \Magento\Framework\App\Console\Request | Mock */ protected $request; /** - * @var \Magento\Framework\ShellInterface|\PHPUnit_Framework_MockObject_MockObject + * @var \Magento\Framework\ShellInterface | Mock */ protected $shell; /** - * @var \Magento\Cron\Model\ResourceModel\Schedule\Collection|\PHPUnit_Framework_MockObject_MockObject + * @var \Magento\Cron\Model\ResourceModel\Schedule\Collection | Mock */ protected $collection; @@ -76,15 +80,25 @@ class ProcessCronQueueObserverTest extends \PHPUnit_Framework_TestCase protected $observer; /** - * @var \Psr\Log\LoggerInterface|\PHPUnit_Framework_MockObject_MockObject + * @var \Psr\Log\LoggerInterface | Mock */ protected $loggerMock; /** - * @var \Magento\Framework\App\State|\PHPUnit_Framework_MockObject_MockObject + * @var \Magento\Framework\App\State | Mock */ protected $appStateMock; + /** + * @var ScheduleResource | Mock + */ + protected $scheduleResource; + + /** + * @var AdapterInterface | Mock + */ + protected $connection; + /** * Prepare parameters */ @@ -100,6 +114,16 @@ protected function setUp() $this->scopeConfig = $this->getMockBuilder( \Magento\Framework\App\Config\ScopeConfigInterface::class )->disableOriginalConstructor()->getMock(); + $this->scopeConfig->expects($this->any())->method('getValue')->will($this->returnValueMap([ + ['system/cron/default/schedule_generate_every', 'store', null, 1], + ['system/cron/default/schedule_ahead_for', 'store', null, 20], + ['system/cron/default/schedule_lifetime', 'store', null, 15], + ['system/cron/default/history_cleanup_every', 'store', null, 10], + ['system/cron/default/history_success_lifetime', 'store', null, 60], + ['system/cron/default/history_failure_lifetime', 'store', null, 600], + ['system/cron/default/use_separate_process', 'store', null, 0], + ])); + $this->collection = $this->getMockBuilder( \Magento\Cron\Model\ResourceModel\Schedule\Collection::class )->setMethods( @@ -107,19 +131,12 @@ protected function setUp() )->disableOriginalConstructor()->getMock(); $this->collection->expects($this->any())->method('addFieldToFilter')->will($this->returnSelf()); $this->collection->expects($this->any())->method('load')->will($this->returnSelf()); - $this->scheduleFactory = $this->getMockBuilder( - \Magento\Cron\Model\ScheduleFactory::class - )->setMethods( - ['create'] - )->disableOriginalConstructor()->getMock(); - $this->request = $this->getMockBuilder( - \Magento\Framework\App\Console\Request::class - )->disableOriginalConstructor()->getMock(); - $this->shell = $this->getMockBuilder( - \Magento\Framework\ShellInterface::class - )->disableOriginalConstructor()->setMethods( - ['execute'] - )->getMock(); + $this->scheduleFactory = $this->getMockBuilder(\Magento\Cron\Model\ScheduleFactory::class) + ->setMethods(['create'])->disableOriginalConstructor()->getMock(); + $this->request = $this->getMockBuilder(\Magento\Framework\App\Console\Request::class) + ->disableOriginalConstructor()->getMock(); + $this->shell = $this->getMockBuilder(\Magento\Framework\ShellInterface::class) + ->disableOriginalConstructor()->setMethods(['execute'])->getMock(); $this->loggerMock = $this->getMock(\Psr\Log\LoggerInterface::class); $this->appStateMock = $this->getMockBuilder(\Magento\Framework\App\State::class) @@ -142,6 +159,13 @@ protected function setUp() ); $phpExecutableFinderFactory->expects($this->any())->method('create')->willReturn($phpExecutableFinder); + $this->scheduleResource = $this->getMockBuilder(ScheduleResource::class) + ->disableOriginalConstructor()->getMock(); + $this->connection = $this->getMockBuilder(AdapterInterface::class)->disableOriginalConstructor()->getMock(); + + $this->scheduleResource->method('getConnection')->willReturn($this->connection); + $this->connection->method('delete')->willReturn(1); + $this->cronQueueObserver = new ProcessCronQueueObserver( $this->objectManager, $this->scheduleFactory, @@ -158,112 +182,101 @@ protected function setUp() } /** - * Test case without saved cron jobs in data base + * Test case for an empty cron_schedule table and no job generation */ public function testDispatchNoPendingJobs() { - $lastRun = time() + 10000000; + $lastRun = time() + 10000000; // skip cleanup and generation $this->cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); - $this->scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(0)); + $this->request->expects($this->any())->method('getParam')->will($this->returnValue('default')); - $this->config->expects($this->once())->method('getJobs')->will($this->returnValue([])); + $this->config->expects($this->exactly(2))->method('getJobs') + ->will($this->returnValue(['default' => ['test_job1' => ['test_data']]])); - $scheduleMock = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class - )->disableOriginalConstructor()->getMock(); + $scheduleMock = $this->getMockBuilder(Schedule::class)->disableOriginalConstructor()->getMock(); $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); - $this->scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); + $scheduleMock->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource)); + $scheduleMock->expects($this->never())->method('setStatus'); + $scheduleMock->expects($this->never())->method('setExecutedAt'); + $this->scheduleFactory->expects($this->exactly(2))->method('create')->will($this->returnValue($scheduleMock)); $this->cronQueueObserver->execute($this->observer); } /** - * Test case for not existed cron jobs in files but in data base is presented + * Test case for a cron job in the database that is not found in the config */ public function testDispatchNoJobConfig() { - $lastRun = time() + 10000000; + $lastRun = time() + 10000000; // skip cleanup and generation $this->cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); - $this->scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(0)); - - $this->config->expects( - $this->once() - )->method( - 'getJobs' - )->will( - $this->returnValue(['test_job1' => ['test_data']]) - ); + $this->request->expects($this->any())->method('getParam')->will($this->returnValue('default')); - $schedule = $this->getMock(\Magento\Cron\Model\Schedule::class, ['getJobCode', '__wakeup'], [], '', false); - $schedule->expects($this->once())->method('getJobCode')->will($this->returnValue('not_existed_job_code')); + $this->config->expects($this->any())->method('getJobs')->will($this->returnValue(['default' => []])); - $this->collection->addItem($schedule); + /** @var Schedule | Mock $schedule */ + $schedule = $this->getMock(Schedule::class, ['getJobCode', '__wakeup'], [], '', false); + $schedule->expects($this->any())->method('getJobCode')->will($this->returnValue('not_existed_job_code')); - $this->config->expects($this->once())->method('getJobs')->will($this->returnValue([])); + $this->collection->addItem($schedule); - $scheduleMock = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class - )->disableOriginalConstructor()->getMock(); + $scheduleMock = $this->getMockBuilder(Schedule::class)->disableOriginalConstructor()->getMock(); $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $scheduleMock->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource)); + $scheduleMock->expects($this->never())->method('getScheduledAt'); $this->scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); $this->cronQueueObserver->execute($this->observer); } /** - * Test case checks if some job can't be locked + * Test case when a job can't be locked */ public function testDispatchCanNotLock() { - $lastRun = time() + 10000000; + $lastRun = time() + 10000000; // skip cleanup and generation $this->cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); - $this->scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(0)); - $this->request->expects($this->any())->method('getParam')->will($this->returnValue('test_group')); - $schedule = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class - )->setMethods( - ['getJobCode', 'tryLockJob', 'getScheduledAt', '__wakeup', 'save'] - )->disableOriginalConstructor()->getMock(); + $this->request->expects($this->any())->method('getParam')->will($this->returnValue('default')); + /** @var Schedule | Mock $schedule */ + $schedule = $this->getMockBuilder(Schedule::class) + ->setMethods(['getJobCode', 'tryLockJob', 'getScheduledAt', '__wakeup', 'save']) + ->disableOriginalConstructor()->getMock(); $schedule->expects($this->any())->method('getJobCode')->will($this->returnValue('test_job1')); $schedule->expects($this->once())->method('getScheduledAt')->will($this->returnValue('-1 day')); $schedule->expects($this->once())->method('tryLockJob')->will($this->returnValue(false)); + $schedule->expects($this->never())->method('setStatus'); + $schedule->expects($this->never())->method('setExecutedAt'); $abstractModel = $this->getMock(\Magento\Framework\Model\AbstractModel::class, [], [], '', false); $schedule->expects($this->any())->method('save')->will($this->returnValue($abstractModel)); $this->collection->addItem($schedule); - $this->config->expects( - $this->once() - )->method( - 'getJobs' - )->will( - $this->returnValue(['test_group' => ['test_job1' => ['test_data']]]) - ); + $this->config->expects($this->exactly(2))->method('getJobs') + ->will($this->returnValue(['default' => ['test_job1' => ['test_data']]])); - $scheduleMock = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class - )->disableOriginalConstructor()->getMock(); + $scheduleMock = $this->getMockBuilder(Schedule::class)->disableOriginalConstructor()->getMock(); $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); - $this->scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); + $scheduleMock->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource)); + $this->scheduleFactory->expects($this->exactly(2))->method('create')->will($this->returnValue($scheduleMock)); $this->cronQueueObserver->execute($this->observer); } /** - * Test case catch exception if to late for schedule + * Test case for catching the exception 'Too late for the schedule' */ public function testDispatchExceptionTooLate() { + $lastRun = time() + 10000000; // skip cleanup and generation $exceptionMessage = 'Too late for the schedule'; $scheduleId = 42; $jobCode = 'test_job1'; $exception = $exceptionMessage . ' Schedule Id: ' . $scheduleId . ' Job Code: ' . $jobCode; - $lastRun = time() + 10000000; $this->cache->expects($this->any())->method('load')->willReturn($lastRun); - $this->scopeConfig->expects($this->any())->method('getValue')->willReturn(0); - $this->request->expects($this->any())->method('getParam')->willReturn('test_group'); + $this->request->expects($this->any())->method('getParam')->willReturn('default'); + /** @var Schedule | Mock $schedule */ $schedule = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class + Schedule::class )->setMethods( [ 'getJobCode', @@ -279,90 +292,68 @@ public function testDispatchExceptionTooLate() ] )->disableOriginalConstructor()->getMock(); $schedule->expects($this->any())->method('getJobCode')->willReturn($jobCode); - $schedule->expects($this->once())->method('getScheduledAt')->willReturn('-1 day'); + $schedule->expects($this->once())->method('getScheduledAt')->willReturn('-16 minutes'); $schedule->expects($this->once())->method('tryLockJob')->willReturn(true); - $schedule->expects( - $this->once() - )->method( - 'setStatus' - )->with( - $this->equalTo(\Magento\Cron\Model\Schedule::STATUS_MISSED) - )->willReturnSelf(); + $schedule->expects($this->once())->method('setStatus') + ->with($this->equalTo(Schedule::STATUS_MISSED))->willReturnSelf(); $schedule->expects($this->once())->method('setMessages')->with($this->equalTo($exceptionMessage)); + $schedule->expects($this->once())->method('setStatus')->with(Schedule::STATUS_MISSED); $schedule->expects($this->any())->method('getStatus')->willReturn(Schedule::STATUS_MISSED); $schedule->expects($this->once())->method('getMessages')->willReturn($exceptionMessage); $schedule->expects($this->once())->method('getScheduleId')->willReturn($scheduleId); $schedule->expects($this->once())->method('save'); + $this->collection->addItem($schedule); $this->appStateMock->expects($this->once())->method('getMode')->willReturn(State::MODE_DEVELOPER); - $this->loggerMock->expects($this->once())->method('info')->with($exception); + $this->config->expects($this->exactly(2))->method('getJobs') + ->willReturn(['default' => ['test_job1' => ['test_data']]]); - $this->collection->addItem($schedule); - - $this->config->expects( - $this->once() - )->method( - 'getJobs' - )->willReturn( - ['test_group' => ['test_job1' => ['test_data']]] - ); - - $scheduleMock = $this->getMockBuilder(\Magento\Cron\Model\Schedule::class) + $scheduleMock = $this->getMockBuilder(Schedule::class) ->disableOriginalConstructor()->getMock(); $scheduleMock->expects($this->any())->method('getCollection')->willReturn($this->collection); - $this->scheduleFactory->expects($this->once())->method('create')->willReturn($scheduleMock); + $scheduleMock->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource)); + $this->scheduleFactory->expects($this->exactly(2))->method('create')->willReturn($scheduleMock); $this->cronQueueObserver->execute($this->observer); } /** - * Test case catch exception if callback not exist + * Test case catch exception if callback does not exist */ public function testDispatchExceptionNoCallback() { + $lastRun = time() + 10000000; // skip cleanup and generation $exceptionMessage = 'No callbacks found'; $exception = new \Exception(__($exceptionMessage)); + $jobConfig = ['default' => ['test_job1' => ['instance' => 'Some_Class', /* 'method' => 'not_set' */]]]; + /** @var Schedule | Mock $schedule */ $schedule = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class + Schedule::class )->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->once())->method('getScheduledAt')->will($this->returnValue('-1 day')); + $schedule->expects($this->once())->method('getScheduledAt')->will($this->returnValue(date('Y-m-d H:i:00'))); $schedule->expects($this->once())->method('tryLockJob')->will($this->returnValue(true)); - $schedule->expects( - $this->once() - )->method( - 'setStatus' - )->with( - $this->equalTo(\Magento\Cron\Model\Schedule::STATUS_ERROR) - )->will( - $this->returnSelf() - ); + $schedule->expects($this->once())->method('setStatus') + ->with($this->equalTo(Schedule::STATUS_ERROR))->will($this->returnSelf()); $schedule->expects($this->once())->method('setMessages')->with($this->equalTo($exceptionMessage)); $schedule->expects($this->any())->method('getStatus')->willReturn(Schedule::STATUS_ERROR); $schedule->expects($this->once())->method('save'); - $this->request->expects($this->any())->method('getParam')->will($this->returnValue('test_group')); + $this->request->expects($this->any())->method('getParam')->will($this->returnValue('default')); $this->collection->addItem($schedule); - $this->loggerMock->expects($this->once())->method('critical')->with($exception); - - $jobConfig = ['test_group' => ['test_job1' => ['instance' => 'Some_Class']]]; - - $this->config->expects($this->once())->method('getJobs')->will($this->returnValue($jobConfig)); - - $lastRun = time() + 10000000; + $this->config->expects($this->exactly(2))->method('getJobs')->will($this->returnValue($jobConfig)); $this->cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); - $this->scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(strtotime('+1 day'))); - $scheduleMock = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class + Schedule::class )->disableOriginalConstructor()->getMock(); $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); - $this->scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); + $scheduleMock->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource)); + $this->scheduleFactory->expects($this->exactly(2))->method('create')->will($this->returnValue($scheduleMock)); $this->cronQueueObserver->execute($this->observer); } @@ -385,44 +376,36 @@ public function testDispatchExceptionInCallback( $saveCalls, $exception ) { - $jobConfig = [ - 'test_group' => [ - 'test_job1' => ['instance' => $cronJobType, 'method' => 'execute'], - ], - ]; + $lastRun = time() + 10000000; // skip cleanup and generation + $jobConfig = ['default' => ['test_job1' => ['instance' => $cronJobType, 'method' => 'execute']]]; - $this->request->expects($this->any())->method('getParam')->will($this->returnValue('test_group')); - $schedule = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class - )->setMethods( + $this->request->expects($this->any())->method('getParam')->will($this->returnValue('default')); + /** @var Schedule | Mock $schedule */ + $schedule = $this->getMockBuilder(Schedule::class)->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->once())->method('getScheduledAt')->will($this->returnValue('-1 day')); + $schedule->expects($this->once())->method('getScheduledAt')->will($this->returnValue(date('Y-m-d H:i:00'))); $schedule->expects($this->once())->method('tryLockJob')->will($this->returnValue(true)); $schedule->expects($this->once()) ->method('setStatus') - ->with($this->equalTo(\Magento\Cron\Model\Schedule::STATUS_ERROR)) + ->with($this->equalTo(Schedule::STATUS_ERROR)) ->will($this->returnSelf()); $schedule->expects($this->once())->method('setMessages')->with($this->equalTo($exceptionMessage)); $schedule->expects($this->any())->method('getStatus')->willReturn(Schedule::STATUS_ERROR); $schedule->expects($this->exactly($saveCalls))->method('save'); - - $this->loggerMock->expects($this->once())->method('critical')->with($exception); - $this->collection->addItem($schedule); - $this->config->expects($this->once())->method('getJobs')->will($this->returnValue($jobConfig)); - - $lastRun = time() + 10000000; + $this->loggerMock->expects($this->once())->method('critical')->with($exception); + $this->config->expects($this->exactly(2))->method('getJobs')->will($this->returnValue($jobConfig)); $this->cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); - $this->scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(strtotime('+1 day'))); $scheduleMock = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class + Schedule::class )->disableOriginalConstructor()->getMock(); $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); - $this->scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); + $scheduleMock->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource)); + $this->scheduleFactory->expects($this->exactly(2))->method('create')->will($this->returnValue($scheduleMock)); $this->objectManager ->expects($this->once()) ->method('create') @@ -460,10 +443,9 @@ public function dispatchExceptionInCallbackDataProvider() */ public function testDispatchRunJob() { - $jobConfig = [ - 'test_group' => ['test_job1' => ['instance' => 'CronJob', 'method' => 'execute']], - ]; - $this->request->expects($this->any())->method('getParam')->will($this->returnValue('test_group')); + $lastRun = time() + 10000000; // skip cleanup and generation + $jobConfig = ['default' => ['test_job1' => ['instance' => 'CronJob', 'method' => 'execute']]]; + $this->request->expects($this->any())->method('getParam')->will($this->returnValue('default')); $scheduleMethods = [ 'getJobCode', @@ -476,14 +458,14 @@ public function testDispatchRunJob() 'setFinishedAt', '__wakeup', ]; - /** @var \Magento\Cron\Model\Schedule|\PHPUnit_Framework_MockObject_MockObject $schedule */ + /** @var Schedule | Mock $schedule */ $schedule = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class + Schedule::class )->setMethods( $scheduleMethods )->disableOriginalConstructor()->getMock(); $schedule->expects($this->any())->method('getJobCode')->will($this->returnValue('test_job1')); - $schedule->expects($this->once())->method('getScheduledAt')->will($this->returnValue('-1 day')); + $schedule->expects($this->once())->method('getScheduledAt')->will($this->returnValue(date('Y-m-d H:i:00'))); $schedule->expects($this->once())->method('tryLockJob')->will($this->returnValue(true)); // cron start to execute some job @@ -491,108 +473,84 @@ public function testDispatchRunJob() $schedule->expects($this->at(5))->method('save'); // cron end execute some job - $schedule->expects( - $this->at(6) - )->method( - 'setStatus' - )->with( - $this->equalTo(\Magento\Cron\Model\Schedule::STATUS_SUCCESS) - )->will( - $this->returnSelf() - ); + $schedule->expects($this->at(6)) + ->method('setStatus') + ->with($this->equalTo(Schedule::STATUS_SUCCESS)) + ->will($this->returnSelf()); $schedule->expects($this->at(8))->method('save'); $this->collection->addItem($schedule); - - $this->config->expects($this->once())->method('getJobs')->will($this->returnValue($jobConfig)); - - $lastRun = time() + 10000000; + $this->config->expects($this->exactly(2))->method('getJobs')->will($this->returnValue($jobConfig)); $this->cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); - $this->scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(strtotime('+1 day'))); $scheduleMock = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class + Schedule::class )->disableOriginalConstructor()->getMock(); $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); - $this->scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); + $scheduleMock->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource)); + $this->scheduleFactory->expects($this->exactly(2))->method('create')->will($this->returnValue($scheduleMock)); $testCronJob = $this->getMockBuilder('CronJob')->setMethods(['execute'])->getMock(); $testCronJob->expects($this->atLeastOnce())->method('execute')->with($schedule); - $this->objectManager->expects( - $this->once() - )->method( - 'create' - )->with( - $this->equalTo('CronJob') - )->will( - $this->returnValue($testCronJob) - ); + $this->objectManager->expects($this->once()) + ->method('create') + ->with($this->equalTo('CronJob')) + ->will($this->returnValue($testCronJob)); $this->cronQueueObserver->execute($this->observer); } /** * Testing generate(), iterate over saved cron jobs + * Generate should not generate any jobs, because they are already in the database */ public function testDispatchNotGenerate() { - $jobConfig = [ - 'test_group' => ['test_job1' => ['instance' => 'CronJob', 'method' => 'execute']], + $jobConfig = ['default' => [ + 'test_job1' => ['instance' => 'CronJob', 'method' => 'execute', 'schedule' => '* * * * *']] ]; - $this->config->expects($this->at(0))->method('getJobs')->will($this->returnValue($jobConfig)); - $this->config->expects( - $this->at(1) - )->method( - 'getJobs' - )->will( - $this->returnValue(['test_group' => []]) - ); - $this->request->expects($this->any())->method('getParam')->will($this->returnValue('test_group')); - $this->cache->expects( - $this->at(0) - )->method( - 'load' - )->with( - $this->equalTo(ProcessCronQueueObserver::CACHE_KEY_LAST_SCHEDULE_GENERATE_AT . 'test_group') - )->will( - $this->returnValue(time() - 10000000) - ); - $this->cache->expects( - $this->at(2) - )->method( - 'load' - )->with( - $this->equalTo(ProcessCronQueueObserver::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . 'test_group') - )->will( - $this->returnValue(time() + 10000000) - ); - - $this->scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(0)); + $this->config->expects($this->any())->method('getJobs')->will($this->returnValue($jobConfig)); + $this->request->expects($this->any())->method('getParam')->will($this->returnValue('default')); + $this->cache->expects($this->any())->method('load')->willReturnMap([ + [ProcessCronQueueObserver::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . 'default', time() + 1000], // skip cleanup + [ProcessCronQueueObserver::CACHE_KEY_LAST_SCHEDULE_GENERATE_AT . 'default', time() - 1000], // do generation + ]); - $schedule = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class - )->setMethods( - ['getJobCode', 'getScheduledAt', '__wakeup'] - )->disableOriginalConstructor()->getMock(); - $schedule->expects($this->any())->method('getJobCode')->will($this->returnValue('job_code1')); - $schedule->expects($this->once())->method('getScheduledAt')->will($this->returnValue('* * * * *')); - - $this->collection->addItem(new \Magento\Framework\DataObject()); - $this->collection->addItem($schedule); + /** @var Schedule | Mock $schedule */ + $schedule = $this->getMockBuilder(Schedule::class) + ->setMethods(['getJobCode', 'getScheduledAt', '__wakeup', 'save']) + ->disableOriginalConstructor()->getMock(); + $schedule->expects($this->any())->method('getJobCode')->will($this->returnValue('test_job1')); + for ($i=0; $i < 20 * 2; $i+=2) { + $minutes = 0.5 * $i; + $schedule->expects($this->at($i))->method('getScheduledAt') + ->will($this->returnValue(date('Y-m-d H:i:00', strtotime("+$minutes minutes")))); + $schedule->expects($this->at($i + 1))->method('getScheduledAt') + ->will($this->returnValue(date('Y-m-d H:i:00', strtotime("+$minutes minutes")))); + $schedule->expects($this->at($minutes))->method('getId')->willReturn($minutes + 1); + $this->collection->addItem($schedule); + } $this->cache->expects($this->any())->method('save'); - $scheduleMock = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class - )->disableOriginalConstructor()->getMock(); + $scheduleMock = $this->getMockBuilder(Schedule::class) + ->setMethods([ + 'getCollection', + 'getResource', + 'trySchedule', + 'save', + ]) + ->disableOriginalConstructor() + ->getMock(); $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $scheduleMock->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource)); + $scheduleMock->expects($this->exactly(20))->method('trySchedule')->willReturn(true); + $scheduleMock->expects($this->never())->method('save'); $this->scheduleFactory->expects($this->any())->method('create')->will($this->returnValue($scheduleMock)); - $this->scheduleFactory->expects($this->any())->method('create')->will($this->returnValue($schedule)); - $this->cronQueueObserver->execute($this->observer); } @@ -603,7 +561,7 @@ public function testDispatchGenerate() { $jobConfig = [ 'default' => [ - 'test_job1' => [ + 'job1' => [ 'instance' => 'CronJob', 'method' => 'execute', ], @@ -620,191 +578,76 @@ public function testDispatchGenerate() $this->config->expects($this->at(0))->method('getJobs')->willReturn($jobConfig); $this->config->expects($this->at(1))->method('getJobs')->willReturn($jobs); $this->request->expects($this->any())->method('getParam')->willReturn('default'); - $this->cache->expects( - $this->at(0) - )->method( - 'load' - )->with( - $this->equalTo(ProcessCronQueueObserver::CACHE_KEY_LAST_SCHEDULE_GENERATE_AT . 'default') - )->willReturn(time() - 10000000); - $this->cache->expects( - $this->at(2) - )->method( - 'load' - )->with( - $this->equalTo(ProcessCronQueueObserver::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . 'default') - )->willReturn(time() + 10000000); - - $this->scopeConfig->expects($this->any())->method('getValue')->willReturnMap( - [ - [ - 'system/cron/default/schedule_generate_every', - \Magento\Store\Model\ScopeInterface::SCOPE_STORE, - null, - 0 - ], - [ - 'system/cron/default/schedule_ahead_for', - \Magento\Store\Model\ScopeInterface::SCOPE_STORE, - null, - 2 - ] - ] - ); + $this->cache->expects($this->any())->method('load')->willReturnMap([ + [ProcessCronQueueObserver::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . 'default', time() + 1000], // skip cleanup + [ProcessCronQueueObserver::CACHE_KEY_LAST_SCHEDULE_GENERATE_AT . 'default', time() - 1000], // do generation + ]); + /** @var Schedule | Mock $schedule */ $schedule = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class + Schedule::class )->setMethods( - ['getJobCode', 'save', 'getScheduledAt', 'unsScheduleId', 'trySchedule', 'getCollection'] + ['getJobCode', 'save', 'getScheduledAt', 'unsScheduleId', 'trySchedule', 'getCollection', 'getResource'] )->disableOriginalConstructor()->getMock(); - $schedule->expects($this->any())->method('getJobCode')->willReturn('job_code1'); - $schedule->expects($this->once())->method('getScheduledAt')->willReturn('* * * * *'); + $schedule->expects($this->any())->method('getJobCode')->willReturn('job1'); + $schedule->expects($this->exactly(2))->method('getScheduledAt')->willReturn('* * * * *'); $schedule->expects($this->any())->method('unsScheduleId')->willReturnSelf(); $schedule->expects($this->any())->method('trySchedule')->willReturnSelf(); $schedule->expects($this->any())->method('getCollection')->willReturn($this->collection); $schedule->expects($this->atLeastOnce())->method('save')->willReturnSelf(); + $schedule->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource)); - $this->collection->addItem(new \Magento\Framework\DataObject()); $this->collection->addItem($schedule); - $this->cache->expects($this->any())->method('save'); - $this->scheduleFactory->expects($this->any())->method('create')->willReturn($schedule); - $this->cronQueueObserver->execute($this->observer); } /** - * Test case without saved cron jobs in data base + * Test case to test the cleanup process */ public function testDispatchCleanup() { - $jobConfig = [ - 'test_group' => ['test_job1' => ['instance' => 'CronJob', 'method' => 'execute']], - ]; - - $schedule = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class - )->disableOriginalConstructor()->setMethods( - ['getExecutedAt', 'getStatus', 'delete', '__wakeup'] - )->getMock(); - $schedule->expects($this->any())->method('getExecutedAt')->will($this->returnValue('-1 day')); - $schedule->expects($this->any())->method('getStatus')->will($this->returnValue('success')); - $this->request->expects($this->any())->method('getParam')->will($this->returnValue('test_group')); - $this->collection->addItem($schedule); - - $this->config->expects($this->once())->method('getJobs')->will($this->returnValue($jobConfig)); - - $this->cache->expects($this->at(0))->method('load')->will($this->returnValue(time() + 10000000)); - $this->cache->expects($this->at(1))->method('load')->will($this->returnValue(time() - 10000000)); - - $this->scopeConfig->expects($this->any())->method('getValue')->will($this->returnValue(0)); - - $scheduleMock = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class - )->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); - $this->scheduleFactory->expects($this->at(0))->method('create')->will($this->returnValue($scheduleMock)); - - $collection = $this->getMockBuilder( - \Magento\Cron\Model\ResourceModel\Schedule\Collection::class - )->setMethods( - ['addFieldToFilter', 'load', '__wakeup'] - )->disableOriginalConstructor()->getMock(); - $collection->expects($this->any())->method('addFieldToFilter')->will($this->returnSelf()); - $collection->expects($this->any())->method('load')->will($this->returnSelf()); - $collection->addItem($schedule); - - $scheduleMock = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class - )->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($collection)); - $this->scheduleFactory->expects($this->at(1))->method('create')->will($this->returnValue($scheduleMock)); + $jobConfig = ['default' => ['test_job1' => ['instance' => 'CronJob', 'method' => 'execute']]]; + $this->config->expects($this->exactly(2))->method('getJobs')->will($this->returnValue($jobConfig)); + $this->request->expects($this->any())->method('getParam')->will($this->returnValue('default')); - $this->cronQueueObserver->execute($this->observer); - } + $this->cache->expects($this->any())->method('load')->willReturnMap([ + // do cleanup + [ProcessCronQueueObserver::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . 'default', time() - 1000], + // skip generation + [ProcessCronQueueObserver::CACHE_KEY_LAST_SCHEDULE_GENERATE_AT . 'default', time() + 1000], + ]); - public function testMissedJobsCleanedInTime() - { - /* 1. Initialize dependencies of generate() method which is called first */ - $jobConfig = [ - 'test_group' => ['test_job1' => ['instance' => 'CronJob', 'method' => 'execute']], + $jobs = [ + ['status' => 'success', 'age' => '-61 minutes', 'delete' => true], + ['status' => 'success', 'age' => '-59 minutes', 'delete' => false], + ['status' => 'missed', 'age' => '-601 minutes', 'delete' => true], + ['status' => 'missed', 'age' => '-509 minutes', 'delete' => false], + ['status' => 'error', 'age' => '-601 minutes', 'delete' => true], + ['status' => 'error', 'age' => '-509 minutes', 'delete' => false], ]; - // This item was scheduled 2 days ago - /** @var \Magento\Cron\Model\Schedule|\PHPUnit_Framework_MockObject_MockObject $schedule1 */ - $schedule1 = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class - )->disableOriginalConstructor()->setMethods( - ['getExecutedAt', 'getScheduledAt', 'getStatus', 'delete', '__wakeup'] - )->getMock(); - $schedule1->expects($this->any())->method('getExecutedAt')->will($this->returnValue(null)); - $schedule1->expects($this->any())->method('getScheduledAt')->will($this->returnValue('-2 day -2 hour')); - $schedule1->expects($this->any())->method('getStatus')->will($this->returnValue(Schedule::STATUS_MISSED)); - //we expect this job be deleted from the list - $schedule1->expects($this->once())->method('delete')->will($this->returnValue(true)); - - // This item was scheduled 1 day ago - $schedule2 = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class - )->disableOriginalConstructor()->setMethods( - ['getExecutedAt', 'getScheduledAt', 'getStatus', 'delete', '__wakeup'] - )->getMock(); - $schedule2->expects($this->any())->method('getExecutedAt')->will($this->returnValue(null)); - $schedule2->expects($this->any())->method('getScheduledAt')->will($this->returnValue('-1 day')); - $schedule2->expects($this->any())->method('getStatus')->will($this->returnValue(Schedule::STATUS_MISSED)); - //we don't expect this job be deleted from the list - $schedule2->expects($this->never())->method('delete'); - - $this->collection->addItem($schedule1); - $this->config->expects($this->once())->method('getJobs')->will($this->returnValue($jobConfig)); - - //get configuration value CACHE_KEY_LAST_HISTORY_CLEANUP_AT in the "_generate()" - $this->cache->expects($this->at(0))->method('load')->will($this->returnValue(time() + 10000000)); - //get configuration value CACHE_KEY_LAST_HISTORY_CLEANUP_AT in the "_cleanup()" - $this->cache->expects($this->at(1))->method('load')->will($this->returnValue(time() - 10000000)); - - $this->scopeConfig->expects($this->at(0))->method('getValue') - ->with($this->equalTo('system/cron/test_group/use_separate_process')) - ->will($this->returnValue(0)); - $this->scopeConfig->expects($this->at(1))->method('getValue') - ->with($this->equalTo('system/cron/test_group/schedule_generate_every')) - ->will($this->returnValue(0)); - $this->scopeConfig->expects($this->at(2))->method('getValue') - ->with($this->equalTo('system/cron/test_group/history_cleanup_every')) - ->will($this->returnValue(0)); - $this->scopeConfig->expects($this->at(3))->method('getValue') - ->with($this->equalTo('system/cron/test_group/schedule_lifetime')) - ->will($this->returnValue(2*24*60)); - $this->scopeConfig->expects($this->at(4))->method('getValue') - ->with($this->equalTo('system/cron/test_group/history_success_lifetime')) - ->will($this->returnValue(0)); - $this->scopeConfig->expects($this->at(5))->method('getValue') - ->with($this->equalTo('system/cron/test_group/history_failure_lifetime')) - ->will($this->returnValue(0)); - - /* 2. Initialize dependencies of cleanup() method which is called second */ - $scheduleMock = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class - )->disableOriginalConstructor()->getMock(); + foreach ($jobs as $job) { + /** @var Schedule | Mock $schedule */ + $schedule = $this->getMockBuilder(Schedule::class) + ->disableOriginalConstructor() + ->setMethods(['getExecutedAt', 'getStatus', 'delete', '__wakeup'])->getMock(); + $schedule->expects($this->any())->method('getExecutedAt')->will($this->returnValue($job['age'])); + $schedule->expects($this->any())->method('getStatus')->will($this->returnValue($job['status'])); + if ($job['delete']) { + $schedule->expects($this->once())->method('delete'); + } else { + $schedule->expects($this->never())->method('delete'); + } + + $this->collection->addItem($schedule); + } + + $scheduleMock = $this->getMockBuilder(Schedule::class)->disableOriginalConstructor()->getMock(); $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); - $this->scheduleFactory->expects($this->at(0))->method('create')->will($this->returnValue($scheduleMock)); - - $collection = $this->getMockBuilder( - \Magento\Cron\Model\ResourceModel\Schedule\Collection::class - )->setMethods( - ['addFieldToFilter', 'load', '__wakeup'] - )->disableOriginalConstructor()->getMock(); - $collection->expects($this->any())->method('addFieldToFilter')->will($this->returnSelf()); - $collection->expects($this->any())->method('load')->will($this->returnSelf()); - $collection->addItem($schedule1); - $collection->addItem($schedule2); - - $scheduleMock = $this->getMockBuilder( - \Magento\Cron\Model\Schedule::class - )->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($collection)); - $this->scheduleFactory->expects($this->at(1))->method('create')->will($this->returnValue($scheduleMock)); + $scheduleMock->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource)); + $this->scheduleFactory->expects($this->any())->method('create')->will($this->returnValue($scheduleMock)); $this->cronQueueObserver->execute($this->observer); } From a92720da6855e37d27314b13005d57af1acedf71 Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Thu, 15 Jun 2017 14:50:22 +0600 Subject: [PATCH 09/15] Make some functions private, because they are not useful outside of this class. The public functions are either useful outside of this class or useful to disable or extend using plugins --- .../Cron/Observer/ProcessCronQueueObserver.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index 138a87563e136..d933b79f4543d 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -374,7 +374,7 @@ public function generate($groupId) * @param string $groupId * @return $this */ - public function generateJobs($jobs, $exists, $groupId) + private function generateJobs($jobs, $exists, $groupId) { foreach ($jobs as $jobCode => $jobConfig) { $cronExpression = $this->getCronExpression($jobConfig); @@ -394,7 +394,7 @@ public function generateJobs($jobs, $exists, $groupId) * @param string $groupId * @return $this */ - public function cleanup($groupId) + private function cleanup($groupId) { $this->cleanupDisabledJobs($groupId); @@ -462,7 +462,7 @@ public function cleanup($groupId) * @param array $jobConfig * @return mixed */ - public function getConfigSchedule($jobConfig) + private function getConfigSchedule($jobConfig) { $cronExpr = $this->scopeConfig->getValue( $jobConfig['config_path'], @@ -479,7 +479,7 @@ public function getConfigSchedule($jobConfig) * @param array $exists * @return void */ - public function saveSchedule($jobCode, $cronExpression, $timeInterval, $exists) + private function saveSchedule($jobCode, $cronExpression, $timeInterval, $exists) { $currentTime = $this->timezone->scopeTimeStamp(); $timeAhead = $currentTime + $timeInterval; @@ -510,7 +510,7 @@ public function saveSchedule($jobCode, $cronExpression, $timeInterval, $exists) * @param int $time * @return Schedule */ - public function generateSchedule($jobCode, $cronExpression, $time) + private function generateSchedule($jobCode, $cronExpression, $time) { $schedule = $this->scheduleFactory->create() ->setCronExpr($cronExpression) @@ -526,7 +526,7 @@ public function generateSchedule($jobCode, $cronExpression, $time) * @param string $groupId * @return int */ - public function getScheduleTimeInterval($groupId) + private function getScheduleTimeInterval($groupId) { $scheduleAheadFor = (int)$this->scopeConfig->getValue( 'system/cron/' . $groupId . '/' . self::XML_PATH_SCHEDULE_AHEAD_FOR, @@ -562,7 +562,7 @@ public function cleanupDisabledJobs($groupId) * @param $jobConfig * @return null|string */ - public function getCronExpression($jobConfig) + private function getCronExpression($jobConfig) { $cronExpression = null; if (isset($jobConfig['config_path'])) { @@ -600,7 +600,7 @@ public function cleanupScheduleMismatches() /** * @return array */ - public function getJobs() + private function getJobs() { if (is_null($this->jobs)) { $this->jobs = $this->config->getJobs(); From 6d40bab820e5ce23595b8b9650879ec716f12935 Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Thu, 15 Jun 2017 16:39:09 +0600 Subject: [PATCH 10/15] Add a unit test that checks if mismatching schedules will be deleted and if disabled jobs will be deleted --- .../Observer/ProcessCronQueueObserverTest.php | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php b/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php index 9ec1a0f41f357..a86069e8b10fb 100644 --- a/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php +++ b/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php @@ -651,4 +651,81 @@ public function testDispatchCleanup() $this->cronQueueObserver->execute($this->observer); } + + /** + * Testing if mismatching schedules will be deleted and + * if disabled jobs will be deleted + */ + public function testDispatchRemoveConfigMismatch() + { + $jobConfig = ['default' => [ + 'test_job1' => ['instance' => 'CronJob', 'method' => 'execute', 'schedule' => '*/10 * * * *'], + 'test_job2' => ['instance' => 'CronJob', 'method' => 'execute', 'schedule' => null] + ]]; + $this->config->expects($this->exactly(2))->method('getJobs')->will($this->returnValue($jobConfig)); + $this->request->expects($this->any())->method('getParam')->will($this->returnValue('default')); + + $this->cache->expects($this->any())->method('load')->willReturnMap([ + // skip cleanup + [ProcessCronQueueObserver::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . 'default', time() + 1000], + // do generation + [ProcessCronQueueObserver::CACHE_KEY_LAST_SCHEDULE_GENERATE_AT . 'default', time() - 1000], + ]); + + $jobs = []; + for ($i = 0; $i<20; $i++) { + $time = date('Y-m-d H:i:00', strtotime("+$i minutes")); + $jobs[] = [ + 'age' => $time, + 'delete' => !preg_match('#0$#', date('i', strtotime($time))) + ]; + } + + foreach ($jobs as $job) { + /** @var Schedule | Mock $schedule */ + $schedule = $this->getMockBuilder(Schedule::class) + ->disableOriginalConstructor() + ->setMethods(['getJobCode', 'getScheduledAt', 'getStatus', 'delete', 'save', '__wakeup'])->getMock(); + $schedule->expects($this->any())->method('getStatus')->will($this->returnValue('pending')); + $schedule->expects($this->any())->method('getJobCode')->will($this->returnValue('test_job1')); + $schedule->expects($this->any())->method('getScheduledAt')->will($this->returnValue($job['age'])); + $this->collection->addItem($schedule); + } + + /** @var Schedule | Mock $schedule */ + $schedule = $this->getMockBuilder(Schedule::class) + ->disableOriginalConstructor() + ->setMethods(['getJobCode', 'getScheduledAt', 'getStatus', 'delete', 'save', '__wakeup'])->getMock(); + $schedule->expects($this->any())->method('getStatus')->will($this->returnValue('pending')); + $schedule->expects($this->any())->method('getJobCode')->will($this->returnValue('test_job2')); + $schedule->expects($this->any())->method('getScheduledAt')->will($this->returnValue(date('Y-m-d H:i:00'))); + $this->collection->addItem($schedule); + + $scheduleMock = $this->getMockBuilder(Schedule::class)->disableOriginalConstructor() + ->setMethods(['save', 'getCollection', 'getResource'])->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->any())->method('create')->will($this->returnValue($scheduleMock)); + + $query = [ + 'status=?' => Schedule::STATUS_PENDING, + 'job_code=?' => 'test_job2', + ]; + $this->connection->expects($this->at(0))->method('delete')->with(null, $query); + + $scheduledAtList = []; + foreach ($jobs as $job) { + if ($job['delete'] === true) { + $scheduledAtList[] = $job['age']; + } + } + $query = [ + 'status=?' => Schedule::STATUS_PENDING, + 'job_code=?' => 'test_job1', + 'scheduled_at in (?)' => $scheduledAtList, + ]; + $this->connection->expects($this->at(1))->method('delete')->with(null, $query); + + $this->cronQueueObserver->execute($this->observer); + } } From 04f9a0b4b31187ada79f2b5f6611b0a070108e16 Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Thu, 15 Jun 2017 19:51:22 +0600 Subject: [PATCH 11/15] fix static test fails --- .../Magento/Cron/Observer/ProcessCronQueueObserver.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index d933b79f4543d..6b5cbc72aa65a 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -541,7 +541,8 @@ private function getScheduleTimeInterval($groupId) * Clean up scheduled jobs that are disabled in the configuration * This can happen when you turn off a cron job in the config and flush the cache * - * @param $groupId + * @param string $groupId + * @return void */ public function cleanupDisabledJobs($groupId) { @@ -559,7 +560,7 @@ public function cleanupDisabledJobs($groupId) } /** - * @param $jobConfig + * @param array $jobConfig * @return null|string */ private function getCronExpression($jobConfig) @@ -602,7 +603,7 @@ public function cleanupScheduleMismatches() */ private function getJobs() { - if (is_null($this->jobs)) { + if ($this->jobs === null) { $this->jobs = $this->config->getJobs(); } return $this->jobs; From d205a5ebe3bc3bc21e39aad11049b984dc60f8d3 Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Thu, 15 Jun 2017 20:00:08 +0600 Subject: [PATCH 12/15] Everything that is not extended can be private --- .../Observer/ProcessCronQueueObserver.php | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index 6b5cbc72aa65a..bb793678408dd 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -59,72 +59,72 @@ class ProcessCronQueueObserver implements ObserverInterface /** * @var \Magento\Cron\Model\ResourceModel\Schedule\Collection */ - protected $pendingSchedules; + private $pendingSchedules; /** * @var \Magento\Cron\Model\ConfigInterface */ - protected $config; + private $config; /** * @var \Magento\Framework\App\ObjectManager */ - protected $objectManager; + private $objectManager; /** * @var \Magento\Framework\App\CacheInterface */ - protected $cache; + private $cache; /** * @var \Magento\Framework\App\Config\ScopeConfigInterface */ - protected $scopeConfig; + private $scopeConfig; /** * @var ScheduleFactory */ - protected $scheduleFactory; + private $scheduleFactory; /** * @var \Magento\Framework\App\Console\Request */ - protected $request; + private $request; /** * @var \Magento\Framework\ShellInterface */ - protected $shell; + private $shell; /** * @var \Magento\Framework\Stdlib\DateTime\TimezoneInterface */ - protected $timezone; + private $timezone; /** * @var \Symfony\Component\Process\PhpExecutableFinder */ - protected $phpExecutableFinder; + private $phpExecutableFinder; /** * @var \Psr\Log\LoggerInterface */ - protected $logger; + private $logger; /** * @var \Magento\Framework\App\State */ - protected $state; + private $state; /** * @var array */ - protected $invalid = []; + private $invalid = []; /** * @var array */ - protected $jobs; + private $jobs; /** * @param \Magento\Framework\ObjectManagerInterface $objectManager From 1427a3840ac094e9ade31c971d93a8bc0ac9ae2e Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Sun, 18 Jun 2017 12:24:47 +0600 Subject: [PATCH 13/15] rollback backword incompatible refactoring --- .../Observer/ProcessCronQueueObserver.php | 120 +++++++++--------- 1 file changed, 60 insertions(+), 60 deletions(-) diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index bb793678408dd..f440fbb594883 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -59,57 +59,57 @@ class ProcessCronQueueObserver implements ObserverInterface /** * @var \Magento\Cron\Model\ResourceModel\Schedule\Collection */ - private $pendingSchedules; + protected $pendingSchedules; /** * @var \Magento\Cron\Model\ConfigInterface */ - private $config; + protected $_config; /** * @var \Magento\Framework\App\ObjectManager */ - private $objectManager; + protected $_objectManager; /** * @var \Magento\Framework\App\CacheInterface */ - private $cache; + protected $_cache; /** * @var \Magento\Framework\App\Config\ScopeConfigInterface */ - private $scopeConfig; + protected $_scopeConfig; /** * @var ScheduleFactory */ - private $scheduleFactory; + protected $_scheduleFactory; /** * @var \Magento\Framework\App\Console\Request */ - private $request; + protected $_request; /** * @var \Magento\Framework\ShellInterface */ - private $shell; + protected $_shell; /** * @var \Magento\Framework\Stdlib\DateTime\TimezoneInterface */ - private $timezone; + protected $timezone; /** * @var \Symfony\Component\Process\PhpExecutableFinder */ - private $phpExecutableFinder; + protected $phpExecutableFinder; /** * @var \Psr\Log\LoggerInterface */ - private $logger; + protected $logger; /** * @var \Magento\Framework\App\State @@ -153,13 +153,13 @@ public function __construct( \Psr\Log\LoggerInterface $logger, \Magento\Framework\App\State $state ) { - $this->objectManager = $objectManager; - $this->scheduleFactory = $scheduleFactory; - $this->cache = $cache; - $this->config = $config; - $this->scopeConfig = $scopeConfig; - $this->request = $request; - $this->shell = $shell; + $this->_objectManager = $objectManager; + $this->_scheduleFactory = $scheduleFactory; + $this->_cache = $cache; + $this->_config = $config; + $this->_scopeConfig = $scopeConfig; + $this->_request = $request; + $this->_shell = $shell; $this->timezone = $timezone; $this->phpExecutableFinder = $phpExecutableFinderFactory->create(); $this->logger = $logger; @@ -179,29 +179,29 @@ public function __construct( */ public function execute(\Magento\Framework\Event\Observer $observer) { - $pendingJobs = $this->getPendingSchedules(); + $pendingJobs = $this->_getPendingSchedules(); $currentTime = $this->timezone->scopeTimeStamp(); - $jobGroupsRoot = $this->config->getJobs(); + $jobGroupsRoot = $this->_config->getJobs(); $phpPath = $this->phpExecutableFinder->find() ?: 'php'; foreach ($jobGroupsRoot as $groupId => $jobsRoot) { - $this->cleanup($groupId); - $this->generate($groupId); - if ($this->request->getParam('group') !== null - && $this->request->getParam('group') !== '\'' . ($groupId) . '\'' - && $this->request->getParam('group') !== $groupId + $this->_cleanup($groupId); + $this->_generate($groupId); + if ($this->_request->getParam('group') !== null + && $this->_request->getParam('group') !== '\'' . ($groupId) . '\'' + && $this->_request->getParam('group') !== $groupId ) { continue; } - if (($this->request->getParam(self::STANDALONE_PROCESS_STARTED) !== '1') && ( - $this->scopeConfig->getValue( + if (($this->_request->getParam(self::STANDALONE_PROCESS_STARTED) !== '1') && ( + $this->_scopeConfig->getValue( 'system/cron/' . $groupId . '/use_separate_process', \Magento\Store\Model\ScopeInterface::SCOPE_STORE ) == 1 ) ) { - $this->shell->execute( + $this->_shell->execute( $phpPath . ' %s cron:run --group=' . $groupId . ' --' . Cli::INPUT_KEY_BOOTSTRAP . '=' . self::STANDALONE_PROCESS_STARTED . '=1', [ @@ -225,7 +225,7 @@ public function execute(\Magento\Framework\Event\Observer $observer) try { if ($schedule->tryLockJob()) { - $this->runJob($scheduledTime, $currentTime, $jobConfig, $schedule, $groupId); + $this->_runJob($scheduledTime, $currentTime, $jobConfig, $schedule, $groupId); } } catch (\Exception $e) { $schedule->setMessages($e->getMessage()); @@ -261,9 +261,9 @@ public function execute(\Magento\Framework\Event\Observer $observer) * @return void * @throws \Exception */ - public function runJob($scheduledTime, $currentTime, $jobConfig, $schedule, $groupId) + protected function _runJob($scheduledTime, $currentTime, $jobConfig, $schedule, $groupId) { - $scheduleLifetime = (int)$this->scopeConfig->getValue( + $scheduleLifetime = (int)$this->_scopeConfig->getValue( 'system/cron/' . $groupId . '/' . self::XML_PATH_SCHEDULE_LIFETIME, \Magento\Store\Model\ScopeInterface::SCOPE_STORE ); @@ -277,7 +277,7 @@ public function runJob($scheduledTime, $currentTime, $jobConfig, $schedule, $gro $schedule->setStatus(Schedule::STATUS_ERROR); throw new \Exception('No callbacks found'); } - $model = $this->objectManager->create($jobConfig['instance']); + $model = $this->_objectManager->create($jobConfig['instance']); $callback = [$model, $jobConfig['method']]; if (!is_callable($callback)) { $schedule->setStatus(Schedule::STATUS_ERROR); @@ -306,10 +306,10 @@ public function runJob($scheduledTime, $currentTime, $jobConfig, $schedule, $gro * * @return \Magento\Cron\Model\ResourceModel\Schedule\Collection */ - public function getPendingSchedules() + protected function _getPendingSchedules() { if (!$this->pendingSchedules) { - $this->pendingSchedules = $this->scheduleFactory->create()->getCollection()->addFieldToFilter( + $this->pendingSchedules = $this->_scheduleFactory->create()->getCollection()->addFieldToFilter( 'status', Schedule::STATUS_PENDING )->load(); @@ -323,13 +323,13 @@ public function getPendingSchedules() * @param string $groupId * @return $this */ - public function generate($groupId) + protected function _generate($groupId) { /** * check if schedule generation is needed */ - $lastRun = (int)$this->cache->load(self::CACHE_KEY_LAST_SCHEDULE_GENERATE_AT . $groupId); - $rawSchedulePeriod = (int)$this->scopeConfig->getValue( + $lastRun = (int)$this->_cache->load(self::CACHE_KEY_LAST_SCHEDULE_GENERATE_AT . $groupId); + $rawSchedulePeriod = (int)$this->_scopeConfig->getValue( 'system/cron/' . $groupId . '/' . self::XML_PATH_SCHEDULE_GENERATE_EVERY, \Magento\Store\Model\ScopeInterface::SCOPE_STORE ); @@ -338,7 +338,7 @@ public function generate($groupId) return $this; } - $schedules = $this->getPendingSchedules(); + $schedules = $this->_getPendingSchedules(); $exists = []; /** @var Schedule $schedule */ foreach ($schedules as $schedule) { @@ -350,13 +350,13 @@ public function generate($groupId) */ $jobs = $this->getJobs(); $this->invalid = []; - $this->generateJobs($jobs[$groupId], $exists, $groupId); + $this->_generateJobs($jobs[$groupId], $exists, $groupId); $this->cleanupScheduleMismatches(); /** * save time schedules generation was ran with no expiration */ - $this->cache->save( + $this->_cache->save( $this->timezone->scopeTimeStamp(), self::CACHE_KEY_LAST_SCHEDULE_GENERATE_AT . $groupId, ['crontab'], @@ -374,7 +374,7 @@ public function generate($groupId) * @param string $groupId * @return $this */ - private function generateJobs($jobs, $exists, $groupId) + protected function _generateJobs($jobs, $exists, $groupId) { foreach ($jobs as $jobCode => $jobConfig) { $cronExpression = $this->getCronExpression($jobConfig); @@ -394,13 +394,13 @@ private function generateJobs($jobs, $exists, $groupId) * @param string $groupId * @return $this */ - private function cleanup($groupId) + protected function _cleanup($groupId) { $this->cleanupDisabledJobs($groupId); // check if history cleanup is needed - $lastCleanup = (int)$this->cache->load(self::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . $groupId); - $historyCleanUp = (int)$this->scopeConfig->getValue( + $lastCleanup = (int)$this->_cache->load(self::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . $groupId); + $historyCleanUp = (int)$this->_scopeConfig->getValue( 'system/cron/' . $groupId . '/' . self::XML_PATH_HISTORY_CLEANUP_EVERY, \Magento\Store\Model\ScopeInterface::SCOPE_STORE ); @@ -409,7 +409,7 @@ private function cleanup($groupId) } // check how long the record should stay unprocessed before marked as MISSED - $scheduleLifetime = (int)$this->scopeConfig->getValue( + $scheduleLifetime = (int)$this->_scopeConfig->getValue( 'system/cron/' . $groupId . '/' . self::XML_PATH_SCHEDULE_LIFETIME, \Magento\Store\Model\ScopeInterface::SCOPE_STORE ); @@ -418,16 +418,16 @@ private function cleanup($groupId) /** * @var \Magento\Cron\Model\ResourceModel\Schedule\Collection $history */ - $history = $this->scheduleFactory->create()->getCollection()->addFieldToFilter( + $history = $this->_scheduleFactory->create()->getCollection()->addFieldToFilter( 'status', ['in' => [Schedule::STATUS_SUCCESS, Schedule::STATUS_MISSED, Schedule::STATUS_ERROR]] )->load(); - $historySuccess = (int)$this->scopeConfig->getValue( + $historySuccess = (int)$this->_scopeConfig->getValue( 'system/cron/' . $groupId . '/' . self::XML_PATH_HISTORY_SUCCESS, \Magento\Store\Model\ScopeInterface::SCOPE_STORE ); - $historyFailure = (int)$this->scopeConfig->getValue( + $historyFailure = (int)$this->_scopeConfig->getValue( 'system/cron/' . $groupId . '/' . self::XML_PATH_HISTORY_FAILURE, \Magento\Store\Model\ScopeInterface::SCOPE_STORE ); @@ -448,7 +448,7 @@ private function cleanup($groupId) } // save time history cleanup was ran with no expiration - $this->cache->save( + $this->_cache->save( $this->timezone->scopeTimeStamp(), self::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . $groupId, ['crontab'], @@ -462,9 +462,9 @@ private function cleanup($groupId) * @param array $jobConfig * @return mixed */ - private function getConfigSchedule($jobConfig) + protected function getConfigSchedule($jobConfig) { - $cronExpr = $this->scopeConfig->getValue( + $cronExpr = $this->_scopeConfig->getValue( $jobConfig['config_path'], \Magento\Store\Model\ScopeInterface::SCOPE_STORE ); @@ -479,7 +479,7 @@ private function getConfigSchedule($jobConfig) * @param array $exists * @return void */ - private function saveSchedule($jobCode, $cronExpression, $timeInterval, $exists) + protected function saveSchedule($jobCode, $cronExpression, $timeInterval, $exists) { $currentTime = $this->timezone->scopeTimeStamp(); $timeAhead = $currentTime + $timeInterval; @@ -510,9 +510,9 @@ private function saveSchedule($jobCode, $cronExpression, $timeInterval, $exists) * @param int $time * @return Schedule */ - private function generateSchedule($jobCode, $cronExpression, $time) + protected function generateSchedule($jobCode, $cronExpression, $time) { - $schedule = $this->scheduleFactory->create() + $schedule = $this->_scheduleFactory->create() ->setCronExpr($cronExpression) ->setJobCode($jobCode) ->setStatus(Schedule::STATUS_PENDING) @@ -526,9 +526,9 @@ private function generateSchedule($jobCode, $cronExpression, $time) * @param string $groupId * @return int */ - private function getScheduleTimeInterval($groupId) + protected function getScheduleTimeInterval($groupId) { - $scheduleAheadFor = (int)$this->scopeConfig->getValue( + $scheduleAheadFor = (int)$this->_scopeConfig->getValue( 'system/cron/' . $groupId . '/' . self::XML_PATH_SCHEDULE_AHEAD_FOR, \Magento\Store\Model\ScopeInterface::SCOPE_STORE ); @@ -550,7 +550,7 @@ public function cleanupDisabledJobs($groupId) foreach ($jobs[$groupId] as $jobCode => $jobConfig) { if (!$this->getCronExpression($jobConfig)) { /** @var \Magento\Cron\Model\ResourceModel\Schedule $scheduleResource */ - $scheduleResource = $this->scheduleFactory->create()->getResource(); + $scheduleResource = $this->_scheduleFactory->create()->getResource(); $scheduleResource->getConnection()->delete($scheduleResource->getMainTable(), [ 'status=?' => Schedule::STATUS_PENDING, 'job_code=?' => $jobCode, @@ -584,11 +584,11 @@ private function getCronExpression($jobConfig) * * @return $this */ - public function cleanupScheduleMismatches() + private function cleanupScheduleMismatches() { foreach ($this->invalid as $jobCode => $scheduledAtList) { /** @var \Magento\Cron\Model\ResourceModel\Schedule $scheduleResource */ - $scheduleResource = $this->scheduleFactory->create()->getResource(); + $scheduleResource = $this->_scheduleFactory->create()->getResource(); $scheduleResource->getConnection()->delete($scheduleResource->getMainTable(), [ 'status=?' => Schedule::STATUS_PENDING, 'job_code=?' => $jobCode, @@ -604,7 +604,7 @@ public function cleanupScheduleMismatches() private function getJobs() { if ($this->jobs === null) { - $this->jobs = $this->config->getJobs(); + $this->jobs = $this->_config->getJobs(); } return $this->jobs; } From 130b576d5a5e3095d7e710034c7fbb0c10ba53db Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Sun, 18 Jun 2017 12:26:59 +0600 Subject: [PATCH 14/15] rollback backword incompatible refactoring --- .../Magento/Cron/Observer/ProcessCronQueueObserver.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index f440fbb594883..02f69c8beae27 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -59,7 +59,7 @@ class ProcessCronQueueObserver implements ObserverInterface /** * @var \Magento\Cron\Model\ResourceModel\Schedule\Collection */ - protected $pendingSchedules; + protected $_pendingSchedules; /** * @var \Magento\Cron\Model\ConfigInterface @@ -109,7 +109,7 @@ class ProcessCronQueueObserver implements ObserverInterface /** * @var \Psr\Log\LoggerInterface */ - protected $logger; + private $logger; /** * @var \Magento\Framework\App\State @@ -308,13 +308,13 @@ protected function _runJob($scheduledTime, $currentTime, $jobConfig, $schedule, */ protected function _getPendingSchedules() { - if (!$this->pendingSchedules) { - $this->pendingSchedules = $this->_scheduleFactory->create()->getCollection()->addFieldToFilter( + if (!$this->_pendingSchedules) { + $this->_pendingSchedules = $this->_scheduleFactory->create()->getCollection()->addFieldToFilter( 'status', Schedule::STATUS_PENDING )->load(); } - return $this->pendingSchedules; + return $this->_pendingSchedules; } /** From 7327a835f1de22acddbf84e6e339cd36b489b80b Mon Sep 17 00:00:00 2001 From: Anton Evers Date: Sun, 18 Jun 2017 12:31:00 +0600 Subject: [PATCH 15/15] rollback backword incompatible refactoring --- .../Observer/ProcessCronQueueObserverTest.php | 226 +++++++++--------- 1 file changed, 113 insertions(+), 113 deletions(-) diff --git a/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php b/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php index a86069e8b10fb..5884c0a6b5b2b 100644 --- a/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php +++ b/app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php @@ -22,52 +22,52 @@ class ProcessCronQueueObserverTest extends \PHPUnit_Framework_TestCase /** * @var ProcessCronQueueObserver */ - protected $cronQueueObserver; + protected $_observer; /** * @var \Magento\Framework\App\ObjectManager | Mock */ - protected $objectManager; + protected $_objectManager; /** * @var Mock */ - protected $cache; + protected $_cache; /** * @var \Magento\Cron\Model\Config | Mock */ - protected $config; + protected $_config; /** * @var \Magento\Cron\Model\ScheduleFactory | Mock */ - protected $scheduleFactory; + protected $_scheduleFactory; /** * @var \Magento\Framework\App\Config\ScopeConfigInterface | Mock */ - protected $scopeConfig; + protected $_scopeConfig; /** * @var \Magento\Framework\App\Console\Request | Mock */ - protected $request; + protected $_request; /** * @var \Magento\Framework\ShellInterface | Mock */ - protected $shell; + protected $_shell; /** * @var \Magento\Cron\Model\ResourceModel\Schedule\Collection | Mock */ - protected $collection; + protected $_collection; /** * @var \Magento\Cron\Model\Groups\Config\Data */ - protected $cronGroupConfig; + protected $_cronGroupConfig; /** * @var \Magento\Framework\Stdlib\DateTime\TimezoneInterface @@ -92,29 +92,29 @@ class ProcessCronQueueObserverTest extends \PHPUnit_Framework_TestCase /** * @var ScheduleResource | Mock */ - protected $scheduleResource; + private $scheduleResource; /** * @var AdapterInterface | Mock */ - protected $connection; + private $connection; /** * Prepare parameters */ protected function setUp() { - $this->objectManager = $this->getMockBuilder( + $this->_objectManager = $this->getMockBuilder( \Magento\Framework\App\ObjectManager::class )->disableOriginalConstructor()->getMock(); - $this->cache = $this->getMock(\Magento\Framework\App\CacheInterface::class); - $this->config = $this->getMockBuilder( + $this->_cache = $this->getMock(\Magento\Framework\App\CacheInterface::class); + $this->_config = $this->getMockBuilder( \Magento\Cron\Model\Config::class )->disableOriginalConstructor()->getMock(); - $this->scopeConfig = $this->getMockBuilder( + $this->_scopeConfig = $this->getMockBuilder( \Magento\Framework\App\Config\ScopeConfigInterface::class )->disableOriginalConstructor()->getMock(); - $this->scopeConfig->expects($this->any())->method('getValue')->will($this->returnValueMap([ + $this->_scopeConfig->expects($this->any())->method('getValue')->will($this->returnValueMap([ ['system/cron/default/schedule_generate_every', 'store', null, 1], ['system/cron/default/schedule_ahead_for', 'store', null, 20], ['system/cron/default/schedule_lifetime', 'store', null, 15], @@ -124,18 +124,18 @@ protected function setUp() ['system/cron/default/use_separate_process', 'store', null, 0], ])); - $this->collection = $this->getMockBuilder( + $this->_collection = $this->getMockBuilder( \Magento\Cron\Model\ResourceModel\Schedule\Collection::class )->setMethods( ['addFieldToFilter', 'load', '__wakeup'] )->disableOriginalConstructor()->getMock(); - $this->collection->expects($this->any())->method('addFieldToFilter')->will($this->returnSelf()); - $this->collection->expects($this->any())->method('load')->will($this->returnSelf()); - $this->scheduleFactory = $this->getMockBuilder(\Magento\Cron\Model\ScheduleFactory::class) + $this->_collection->expects($this->any())->method('addFieldToFilter')->will($this->returnSelf()); + $this->_collection->expects($this->any())->method('load')->will($this->returnSelf()); + $this->_scheduleFactory = $this->getMockBuilder(\Magento\Cron\Model\ScheduleFactory::class) ->setMethods(['create'])->disableOriginalConstructor()->getMock(); - $this->request = $this->getMockBuilder(\Magento\Framework\App\Console\Request::class) + $this->_request = $this->getMockBuilder(\Magento\Framework\App\Console\Request::class) ->disableOriginalConstructor()->getMock(); - $this->shell = $this->getMockBuilder(\Magento\Framework\ShellInterface::class) + $this->_shell = $this->getMockBuilder(\Magento\Framework\ShellInterface::class) ->disableOriginalConstructor()->setMethods(['execute'])->getMock(); $this->loggerMock = $this->getMock(\Psr\Log\LoggerInterface::class); @@ -166,14 +166,14 @@ protected function setUp() $this->scheduleResource->method('getConnection')->willReturn($this->connection); $this->connection->method('delete')->willReturn(1); - $this->cronQueueObserver = new ProcessCronQueueObserver( - $this->objectManager, - $this->scheduleFactory, - $this->cache, - $this->config, - $this->scopeConfig, - $this->request, - $this->shell, + $this->_observer = new ProcessCronQueueObserver( + $this->_objectManager, + $this->_scheduleFactory, + $this->_cache, + $this->_config, + $this->_scopeConfig, + $this->_request, + $this->_shell, $this->timezone, $phpExecutableFinderFactory, $this->loggerMock, @@ -187,20 +187,20 @@ protected function setUp() public function testDispatchNoPendingJobs() { $lastRun = time() + 10000000; // skip cleanup and generation - $this->cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); - $this->request->expects($this->any())->method('getParam')->will($this->returnValue('default')); + $this->_cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); + $this->_request->expects($this->any())->method('getParam')->will($this->returnValue('default')); - $this->config->expects($this->exactly(2))->method('getJobs') + $this->_config->expects($this->exactly(2))->method('getJobs') ->will($this->returnValue(['default' => ['test_job1' => ['test_data']]])); $scheduleMock = $this->getMockBuilder(Schedule::class)->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->_collection)); $scheduleMock->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource)); $scheduleMock->expects($this->never())->method('setStatus'); $scheduleMock->expects($this->never())->method('setExecutedAt'); - $this->scheduleFactory->expects($this->exactly(2))->method('create')->will($this->returnValue($scheduleMock)); + $this->_scheduleFactory->expects($this->exactly(2))->method('create')->will($this->returnValue($scheduleMock)); - $this->cronQueueObserver->execute($this->observer); + $this->_observer->execute($this->observer); } /** @@ -209,24 +209,24 @@ public function testDispatchNoPendingJobs() public function testDispatchNoJobConfig() { $lastRun = time() + 10000000; // skip cleanup and generation - $this->cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); - $this->request->expects($this->any())->method('getParam')->will($this->returnValue('default')); + $this->_cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); + $this->_request->expects($this->any())->method('getParam')->will($this->returnValue('default')); - $this->config->expects($this->any())->method('getJobs')->will($this->returnValue(['default' => []])); + $this->_config->expects($this->any())->method('getJobs')->will($this->returnValue(['default' => []])); /** @var Schedule | Mock $schedule */ $schedule = $this->getMock(Schedule::class, ['getJobCode', '__wakeup'], [], '', false); $schedule->expects($this->any())->method('getJobCode')->will($this->returnValue('not_existed_job_code')); - $this->collection->addItem($schedule); + $this->_collection->addItem($schedule); $scheduleMock = $this->getMockBuilder(Schedule::class)->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->_collection)); $scheduleMock->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource)); $scheduleMock->expects($this->never())->method('getScheduledAt'); - $this->scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); + $this->_scheduleFactory->expects($this->once())->method('create')->will($this->returnValue($scheduleMock)); - $this->cronQueueObserver->execute($this->observer); + $this->_observer->execute($this->observer); } /** @@ -235,8 +235,8 @@ public function testDispatchNoJobConfig() public function testDispatchCanNotLock() { $lastRun = time() + 10000000; // skip cleanup and generation - $this->cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); - $this->request->expects($this->any())->method('getParam')->will($this->returnValue('default')); + $this->_cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); + $this->_request->expects($this->any())->method('getParam')->will($this->returnValue('default')); /** @var Schedule | Mock $schedule */ $schedule = $this->getMockBuilder(Schedule::class) ->setMethods(['getJobCode', 'tryLockJob', 'getScheduledAt', '__wakeup', 'save']) @@ -248,17 +248,17 @@ public function testDispatchCanNotLock() $schedule->expects($this->never())->method('setExecutedAt'); $abstractModel = $this->getMock(\Magento\Framework\Model\AbstractModel::class, [], [], '', false); $schedule->expects($this->any())->method('save')->will($this->returnValue($abstractModel)); - $this->collection->addItem($schedule); + $this->_collection->addItem($schedule); - $this->config->expects($this->exactly(2))->method('getJobs') + $this->_config->expects($this->exactly(2))->method('getJobs') ->will($this->returnValue(['default' => ['test_job1' => ['test_data']]])); $scheduleMock = $this->getMockBuilder(Schedule::class)->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $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(2))->method('create')->will($this->returnValue($scheduleMock)); - $this->cronQueueObserver->execute($this->observer); + $this->_observer->execute($this->observer); } /** @@ -272,8 +272,8 @@ public function testDispatchExceptionTooLate() $jobCode = 'test_job1'; $exception = $exceptionMessage . ' Schedule Id: ' . $scheduleId . ' Job Code: ' . $jobCode; - $this->cache->expects($this->any())->method('load')->willReturn($lastRun); - $this->request->expects($this->any())->method('getParam')->willReturn('default'); + $this->_cache->expects($this->any())->method('load')->willReturn($lastRun); + $this->_request->expects($this->any())->method('getParam')->willReturn('default'); /** @var Schedule | Mock $schedule */ $schedule = $this->getMockBuilder( Schedule::class @@ -302,20 +302,20 @@ public function testDispatchExceptionTooLate() $schedule->expects($this->once())->method('getMessages')->willReturn($exceptionMessage); $schedule->expects($this->once())->method('getScheduleId')->willReturn($scheduleId); $schedule->expects($this->once())->method('save'); - $this->collection->addItem($schedule); + $this->_collection->addItem($schedule); $this->appStateMock->expects($this->once())->method('getMode')->willReturn(State::MODE_DEVELOPER); $this->loggerMock->expects($this->once())->method('info')->with($exception); - $this->config->expects($this->exactly(2))->method('getJobs') + $this->_config->expects($this->exactly(2))->method('getJobs') ->willReturn(['default' => ['test_job1' => ['test_data']]]); $scheduleMock = $this->getMockBuilder(Schedule::class) ->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->willReturn($this->collection); + $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(2))->method('create')->willReturn($scheduleMock); - $this->cronQueueObserver->execute($this->observer); + $this->_observer->execute($this->observer); } /** @@ -342,20 +342,20 @@ public function testDispatchExceptionNoCallback() $schedule->expects($this->once())->method('setMessages')->with($this->equalTo($exceptionMessage)); $schedule->expects($this->any())->method('getStatus')->willReturn(Schedule::STATUS_ERROR); $schedule->expects($this->once())->method('save'); - $this->request->expects($this->any())->method('getParam')->will($this->returnValue('default')); - $this->collection->addItem($schedule); + $this->_request->expects($this->any())->method('getParam')->will($this->returnValue('default')); + $this->_collection->addItem($schedule); $this->loggerMock->expects($this->once())->method('critical')->with($exception); - $this->config->expects($this->exactly(2))->method('getJobs')->will($this->returnValue($jobConfig)); - $this->cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); + $this->_config->expects($this->exactly(2))->method('getJobs')->will($this->returnValue($jobConfig)); + $this->_cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); $scheduleMock = $this->getMockBuilder( Schedule::class )->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $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(2))->method('create')->will($this->returnValue($scheduleMock)); - $this->cronQueueObserver->execute($this->observer); + $this->_observer->execute($this->observer); } /** @@ -379,7 +379,7 @@ public function testDispatchExceptionInCallback( $lastRun = time() + 10000000; // skip cleanup and generation $jobConfig = ['default' => ['test_job1' => ['instance' => $cronJobType, 'method' => 'execute']]]; - $this->request->expects($this->any())->method('getParam')->will($this->returnValue('default')); + $this->_request->expects($this->any())->method('getParam')->will($this->returnValue('default')); /** @var Schedule | Mock $schedule */ $schedule = $this->getMockBuilder(Schedule::class)->setMethods( ['getJobCode', 'tryLockJob', 'getScheduledAt', 'save', 'setStatus', 'setMessages', '__wakeup', 'getStatus'] @@ -394,25 +394,25 @@ public function testDispatchExceptionInCallback( $schedule->expects($this->once())->method('setMessages')->with($this->equalTo($exceptionMessage)); $schedule->expects($this->any())->method('getStatus')->willReturn(Schedule::STATUS_ERROR); $schedule->expects($this->exactly($saveCalls))->method('save'); - $this->collection->addItem($schedule); + $this->_collection->addItem($schedule); $this->loggerMock->expects($this->once())->method('critical')->with($exception); - $this->config->expects($this->exactly(2))->method('getJobs')->will($this->returnValue($jobConfig)); - $this->cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); + $this->_config->expects($this->exactly(2))->method('getJobs')->will($this->returnValue($jobConfig)); + $this->_cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); $scheduleMock = $this->getMockBuilder( Schedule::class )->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $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->objectManager + $this->_scheduleFactory->expects($this->exactly(2))->method('create')->will($this->returnValue($scheduleMock)); + $this->_objectManager ->expects($this->once()) ->method('create') ->with($this->equalTo($cronJobType)) ->will($this->returnValue($cronJobObject)); - $this->cronQueueObserver->execute($this->observer); + $this->_observer->execute($this->observer); } /** @@ -445,7 +445,7 @@ public function testDispatchRunJob() { $lastRun = time() + 10000000; // skip cleanup and generation $jobConfig = ['default' => ['test_job1' => ['instance' => 'CronJob', 'method' => 'execute']]]; - $this->request->expects($this->any())->method('getParam')->will($this->returnValue('default')); + $this->_request->expects($this->any())->method('getParam')->will($this->returnValue('default')); $scheduleMethods = [ 'getJobCode', @@ -480,26 +480,26 @@ public function testDispatchRunJob() $schedule->expects($this->at(8))->method('save'); - $this->collection->addItem($schedule); - $this->config->expects($this->exactly(2))->method('getJobs')->will($this->returnValue($jobConfig)); - $this->cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); + $this->_collection->addItem($schedule); + $this->_config->expects($this->exactly(2))->method('getJobs')->will($this->returnValue($jobConfig)); + $this->_cache->expects($this->any())->method('load')->will($this->returnValue($lastRun)); $scheduleMock = $this->getMockBuilder( Schedule::class )->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $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(2))->method('create')->will($this->returnValue($scheduleMock)); $testCronJob = $this->getMockBuilder('CronJob')->setMethods(['execute'])->getMock(); $testCronJob->expects($this->atLeastOnce())->method('execute')->with($schedule); - $this->objectManager->expects($this->once()) + $this->_objectManager->expects($this->once()) ->method('create') ->with($this->equalTo('CronJob')) ->will($this->returnValue($testCronJob)); - $this->cronQueueObserver->execute($this->observer); + $this->_observer->execute($this->observer); } /** @@ -512,9 +512,9 @@ public function testDispatchNotGenerate() 'test_job1' => ['instance' => 'CronJob', 'method' => 'execute', 'schedule' => '* * * * *']] ]; - $this->config->expects($this->any())->method('getJobs')->will($this->returnValue($jobConfig)); - $this->request->expects($this->any())->method('getParam')->will($this->returnValue('default')); - $this->cache->expects($this->any())->method('load')->willReturnMap([ + $this->_config->expects($this->any())->method('getJobs')->will($this->returnValue($jobConfig)); + $this->_request->expects($this->any())->method('getParam')->will($this->returnValue('default')); + $this->_cache->expects($this->any())->method('load')->willReturnMap([ [ProcessCronQueueObserver::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . 'default', time() + 1000], // skip cleanup [ProcessCronQueueObserver::CACHE_KEY_LAST_SCHEDULE_GENERATE_AT . 'default', time() - 1000], // do generation ]); @@ -531,10 +531,10 @@ public function testDispatchNotGenerate() $schedule->expects($this->at($i + 1))->method('getScheduledAt') ->will($this->returnValue(date('Y-m-d H:i:00', strtotime("+$minutes minutes")))); $schedule->expects($this->at($minutes))->method('getId')->willReturn($minutes + 1); - $this->collection->addItem($schedule); + $this->_collection->addItem($schedule); } - $this->cache->expects($this->any())->method('save'); + $this->_cache->expects($this->any())->method('save'); $scheduleMock = $this->getMockBuilder(Schedule::class) ->setMethods([ @@ -545,13 +545,13 @@ public function testDispatchNotGenerate() ]) ->disableOriginalConstructor() ->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->_collection)); $scheduleMock->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource)); $scheduleMock->expects($this->exactly(20))->method('trySchedule')->willReturn(true); $scheduleMock->expects($this->never())->method('save'); - $this->scheduleFactory->expects($this->any())->method('create')->will($this->returnValue($scheduleMock)); + $this->_scheduleFactory->expects($this->any())->method('create')->will($this->returnValue($scheduleMock)); - $this->cronQueueObserver->execute($this->observer); + $this->_observer->execute($this->observer); } /** @@ -575,10 +575,10 @@ public function testDispatchGenerate() 'job3' => ['schedule' => '* * * * *'], ], ]; - $this->config->expects($this->at(0))->method('getJobs')->willReturn($jobConfig); - $this->config->expects($this->at(1))->method('getJobs')->willReturn($jobs); - $this->request->expects($this->any())->method('getParam')->willReturn('default'); - $this->cache->expects($this->any())->method('load')->willReturnMap([ + $this->_config->expects($this->at(0))->method('getJobs')->willReturn($jobConfig); + $this->_config->expects($this->at(1))->method('getJobs')->willReturn($jobs); + $this->_request->expects($this->any())->method('getParam')->willReturn('default'); + $this->_cache->expects($this->any())->method('load')->willReturnMap([ [ProcessCronQueueObserver::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . 'default', time() + 1000], // skip cleanup [ProcessCronQueueObserver::CACHE_KEY_LAST_SCHEDULE_GENERATE_AT . 'default', time() - 1000], // do generation ]); @@ -593,14 +593,14 @@ public function testDispatchGenerate() $schedule->expects($this->exactly(2))->method('getScheduledAt')->willReturn('* * * * *'); $schedule->expects($this->any())->method('unsScheduleId')->willReturnSelf(); $schedule->expects($this->any())->method('trySchedule')->willReturnSelf(); - $schedule->expects($this->any())->method('getCollection')->willReturn($this->collection); + $schedule->expects($this->any())->method('getCollection')->willReturn($this->_collection); $schedule->expects($this->atLeastOnce())->method('save')->willReturnSelf(); $schedule->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource)); - $this->collection->addItem($schedule); - $this->cache->expects($this->any())->method('save'); - $this->scheduleFactory->expects($this->any())->method('create')->willReturn($schedule); - $this->cronQueueObserver->execute($this->observer); + $this->_collection->addItem($schedule); + $this->_cache->expects($this->any())->method('save'); + $this->_scheduleFactory->expects($this->any())->method('create')->willReturn($schedule); + $this->_observer->execute($this->observer); } /** @@ -609,10 +609,10 @@ public function testDispatchGenerate() public function testDispatchCleanup() { $jobConfig = ['default' => ['test_job1' => ['instance' => 'CronJob', 'method' => 'execute']]]; - $this->config->expects($this->exactly(2))->method('getJobs')->will($this->returnValue($jobConfig)); - $this->request->expects($this->any())->method('getParam')->will($this->returnValue('default')); + $this->_config->expects($this->exactly(2))->method('getJobs')->will($this->returnValue($jobConfig)); + $this->_request->expects($this->any())->method('getParam')->will($this->returnValue('default')); - $this->cache->expects($this->any())->method('load')->willReturnMap([ + $this->_cache->expects($this->any())->method('load')->willReturnMap([ // do cleanup [ProcessCronQueueObserver::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . 'default', time() - 1000], // skip generation @@ -641,15 +641,15 @@ public function testDispatchCleanup() $schedule->expects($this->never())->method('delete'); } - $this->collection->addItem($schedule); + $this->_collection->addItem($schedule); } $scheduleMock = $this->getMockBuilder(Schedule::class)->disableOriginalConstructor()->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $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->any())->method('create')->will($this->returnValue($scheduleMock)); + $this->_scheduleFactory->expects($this->any())->method('create')->will($this->returnValue($scheduleMock)); - $this->cronQueueObserver->execute($this->observer); + $this->_observer->execute($this->observer); } /** @@ -662,10 +662,10 @@ public function testDispatchRemoveConfigMismatch() 'test_job1' => ['instance' => 'CronJob', 'method' => 'execute', 'schedule' => '*/10 * * * *'], 'test_job2' => ['instance' => 'CronJob', 'method' => 'execute', 'schedule' => null] ]]; - $this->config->expects($this->exactly(2))->method('getJobs')->will($this->returnValue($jobConfig)); - $this->request->expects($this->any())->method('getParam')->will($this->returnValue('default')); + $this->_config->expects($this->exactly(2))->method('getJobs')->will($this->returnValue($jobConfig)); + $this->_request->expects($this->any())->method('getParam')->will($this->returnValue('default')); - $this->cache->expects($this->any())->method('load')->willReturnMap([ + $this->_cache->expects($this->any())->method('load')->willReturnMap([ // skip cleanup [ProcessCronQueueObserver::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . 'default', time() + 1000], // do generation @@ -689,7 +689,7 @@ public function testDispatchRemoveConfigMismatch() $schedule->expects($this->any())->method('getStatus')->will($this->returnValue('pending')); $schedule->expects($this->any())->method('getJobCode')->will($this->returnValue('test_job1')); $schedule->expects($this->any())->method('getScheduledAt')->will($this->returnValue($job['age'])); - $this->collection->addItem($schedule); + $this->_collection->addItem($schedule); } /** @var Schedule | Mock $schedule */ @@ -699,13 +699,13 @@ public function testDispatchRemoveConfigMismatch() $schedule->expects($this->any())->method('getStatus')->will($this->returnValue('pending')); $schedule->expects($this->any())->method('getJobCode')->will($this->returnValue('test_job2')); $schedule->expects($this->any())->method('getScheduledAt')->will($this->returnValue(date('Y-m-d H:i:00'))); - $this->collection->addItem($schedule); + $this->_collection->addItem($schedule); $scheduleMock = $this->getMockBuilder(Schedule::class)->disableOriginalConstructor() ->setMethods(['save', 'getCollection', 'getResource'])->getMock(); - $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->collection)); + $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->any())->method('create')->will($this->returnValue($scheduleMock)); + $this->_scheduleFactory->expects($this->any())->method('create')->will($this->returnValue($scheduleMock)); $query = [ 'status=?' => Schedule::STATUS_PENDING, @@ -726,6 +726,6 @@ public function testDispatchRemoveConfigMismatch() ]; $this->connection->expects($this->at(1))->method('delete')->with(null, $query); - $this->cronQueueObserver->execute($this->observer); + $this->_observer->execute($this->observer); } }