From 94fb923f0e6406716784ba8cc8cb7dc90b56da34 Mon Sep 17 00:00:00 2001 From: Luke Rodgers Date: Fri, 17 Sep 2021 16:32:07 +0100 Subject: [PATCH 1/8] Add --exclude-group to cron:run https://devdocs.magento.com/guides/v2.4/config-guide/cli/config-cli-subcommands-cron.html#config-cli-cron-group-run When you decide to split a single cron group with the `--group` flag it means you need to do so for every group in your instance. It's pretty common for third party modules to define their own groups whenever they like, so we have to keep up to date with the defined groups in the instance to keep adding these to our crontab when required. One of my instances currently has 11 groups to manage, more will come in the future. In my case I only really want to run the crons like ``` * * * * * bin/magento cron:run --exclude-group index * * * * * flock -n index.lock bin/magento cron:run --group index ``` This will give me better granular control over the indexer crons which can be useful when your store reaches hundreds of thousands of SKUs For future proofing I've made the `--exclude-group` flag stackable so you can define it multiple times like ``` * * * * * bin/magento cron:run --exclude-group index --exclude-group consumers ``` --- .../Cron/Console/Command/CronCommand.php | 12 +++ .../Observer/ProcessCronQueueObserver.php | 15 +++ .../Observer/ProcessCronQueueObserverTest.php | 99 +++++++++++++++++++ 3 files changed, 126 insertions(+) diff --git a/app/code/Magento/Cron/Console/Command/CronCommand.php b/app/code/Magento/Cron/Console/Command/CronCommand.php index 142a9a397eb5f..28e969df52f59 100644 --- a/app/code/Magento/Cron/Console/Command/CronCommand.php +++ b/app/code/Magento/Cron/Console/Command/CronCommand.php @@ -29,6 +29,11 @@ class CronCommand extends Command */ const INPUT_KEY_GROUP = 'group'; + /** + * Name of input option + */ + const INPUT_KEY_EXCLUDE_GROUP = 'exclude-group'; + /** * Object manager factory * @@ -70,6 +75,12 @@ protected function configure() InputOption::VALUE_REQUIRED, 'Run jobs only from specified group' ), + new InputOption( + self::INPUT_KEY_EXCLUDE_GROUP, + null, + InputOption::VALUE_OPTIONAL | InputOption::VALUE_IS_ARRAY, + 'Exclude jobs from the specified group' + ), new InputOption( Cli::INPUT_KEY_BOOTSTRAP, null, @@ -100,6 +111,7 @@ protected function execute(InputInterface $input, OutputInterface $output) $objectManager = $this->objectManagerFactory->create($omParams); $params[self::INPUT_KEY_GROUP] = $input->getOption(self::INPUT_KEY_GROUP); + $params[self::INPUT_KEY_EXCLUDE_GROUP] = $input->getOption(self::INPUT_KEY_EXCLUDE_GROUP); $params[ProcessCronQueueObserver::STANDALONE_PROCESS_STARTED] = '0'; $bootstrap = $input->getOption(Cli::INPUT_KEY_BOOTSTRAP); if ($bootstrap) { diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index 0f266b5d62d83..a46f823cb28a2 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -250,6 +250,9 @@ function ($a, $b) { if (!$this->isGroupInFilter($groupId)) { continue; } + if ($this->isGroupInExcludeFilter($groupId)) { + continue; + } if ($this->_request->getParam(self::STANDALONE_PROCESS_STARTED) !== '1' && $this->getCronGroupConfigurationValue($groupId, 'use_separate_process') == 1 ) { @@ -795,6 +798,18 @@ private function isGroupInFilter($groupId): bool && trim($this->_request->getParam('group'), "'") !== $groupId); } + /** + * Is Group In Exclude Filter. + * + * @param string $groupId + * @return bool + */ + private function isGroupInExcludeFilter($groupId): bool + { + $excludeGroup = $this->_request->getParam('exclude-group', []); + return in_array($groupId, $excludeGroup); + } + /** * Process pending jobs. * diff --git a/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php b/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php index 4ca8ab53ffbad..a9412c7013c62 100644 --- a/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php +++ b/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php @@ -5,6 +5,7 @@ */ namespace Magento\Cron\Observer; +use Magento\Cron\Observer\ProcessCronQueueObserver; use \Magento\TestFramework\Helper\Bootstrap; class ProcessCronQueueObserverTest extends \PHPUnit\Framework\TestCase @@ -49,4 +50,102 @@ public function testDispatchNoFailed() $this->fail($item->getMessages()); } } + + /** + * @param array $expectedGroupsToRun + * @param null $group + * @param null $excludeGroup + * @dataProvider groupFiltersDataProvider + */ + public function testGroupFilters(array $expectedGroupsToRun, $group = null, $excludeGroup = null) + { + $request = Bootstrap::getObjectManager()->get(\Magento\Framework\App\Console\Request::class); + $lockManager = $this->createMock(\Magento\Framework\Lock\LockManagerInterface::class); + + // The jobs are locked when they are run, assert on them to see which groups would run + $expectedLockData = []; + foreach ($expectedGroupsToRun as $expectedGroupToRun) { + $expectedLockData[] = [ + ProcessCronQueueObserver::LOCK_PREFIX . $expectedGroupToRun, + ProcessCronQueueObserver::LOCK_TIMEOUT + ]; + } + + // No expected lock data, means we should never call it + if (empty($expectedLockData)){ + $lockManager->expects($this->never()) + ->method('lock'); + } + + $lockManager->expects($this->exactly(count($expectedLockData))) + ->method('lock') + ->withConsecutive(...$expectedLockData); + + $request->setParams( + [ + 'group' => $group, + 'exclude-group' => $excludeGroup, + 'standaloneProcessStarted' => '1' + ] + ); + $this->_model = Bootstrap::getObjectManager() + ->create(\Magento\Cron\Observer\ProcessCronQueueObserver::class, [ + 'request' => $request, + 'lockManager' => $lockManager + ]); + $this->_model->execute(new \Magento\Framework\Event\Observer()); + } + + /** + * @return array|array[] + */ + public function groupFiltersDataProvider(): array + { + $listOfGroups = []; + $config = Bootstrap::getObjectManager()->get(\Magento\Cron\Model\ConfigInterface::class); + foreach (array_keys($config->getJobs()) as $groupId) { + $listOfGroups[$groupId] = $groupId; + } + $listOfGroups = array_reverse($listOfGroups, true); + + return [ + 'no flags runs all groups' => [ + $listOfGroups // groups to run + ], + '--group=default should run' => [ + ['default'], // groups to run + 'default', // --group default + ], + '--group=default with --exclude-group=default, nothing should run' => [ + [], // groups to run + 'default', // --group default + ['default'], // --exclude-group default + ], + '--group=default with --exclude-group=index, default should run' => [ + ['default'], // groups to run + 'default', // --group default + ['index'], // --exclude-group index + ], + '--group=index with --exclude-group=default, index should run' => [ + ['index'], // groups to run + 'index', // --group index + ['default'], // --exclude-group default + ], + '--exclude-group=index, all other groups should run' => [ + array_filter($listOfGroups, function($g) { return $g !== 'index'; }), // groups to run, all but index + null, // + ['index'] // --exclude-group index + ], + '--exclude-group for every group runs nothing' => [ + [], // groups to run, none + null, // + $listOfGroups // groups to exclude, all of them + ], + 'exclude all groups but consumers, consumers runs' => [ + array_filter($listOfGroups, function($g) { return $g === 'consumers'; }), + null, + array_filter($listOfGroups, function($g) { return $g !== 'consumers'; }) + ], + ]; + } } From 03654f83217f2d01c76cd0dbef499abfd7c477bb Mon Sep 17 00:00:00 2001 From: Luke Rodgers Date: Fri, 17 Sep 2021 17:30:13 +0100 Subject: [PATCH 2/8] Fix unit test --- app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php index a46f823cb28a2..c186d95b1c30b 100644 --- a/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php +++ b/app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php @@ -807,7 +807,7 @@ private function isGroupInFilter($groupId): bool private function isGroupInExcludeFilter($groupId): bool { $excludeGroup = $this->_request->getParam('exclude-group', []); - return in_array($groupId, $excludeGroup); + return is_array($excludeGroup) && in_array($groupId, $excludeGroup); } /** From 52580cc8aa29a3ee6cbe0110b2b7f3e7e5d3e592 Mon Sep 17 00:00:00 2001 From: Luke Rodgers Date: Thu, 23 Sep 2021 09:10:22 +0100 Subject: [PATCH 3/8] Fix static analysis --- .../Magento/Cron/Console/Command/CronCommand.php | 2 ++ .../Cron/Observer/ProcessCronQueueObserverTest.php | 14 ++++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/Cron/Console/Command/CronCommand.php b/app/code/Magento/Cron/Console/Command/CronCommand.php index 28e969df52f59..b3a5f570e939f 100644 --- a/app/code/Magento/Cron/Console/Command/CronCommand.php +++ b/app/code/Magento/Cron/Console/Command/CronCommand.php @@ -105,6 +105,7 @@ protected function execute(InputInterface $input, OutputInterface $output) $output->writeln('' . 'Cron is disabled. Jobs were not run.' . ''); return; } + // phpcs:ignore Magento2.Security.Superglobal.SuperglobalUsageWarning $omParams = $_SERVER; $omParams[StoreManager::PARAM_RUN_CODE] = 'admin'; $omParams[Store::CUSTOM_ENTRY_POINT_PARAM] = true; @@ -128,5 +129,6 @@ protected function execute(InputInterface $input, OutputInterface $output) $cronObserver = $objectManager->create(\Magento\Framework\App\Cron::class, ['parameters' => $params]); $cronObserver->launch(); $output->writeln('' . 'Ran jobs by schedule.' . ''); + return Cli::RETURN_SUCCESS; } } diff --git a/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php b/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php index a9412c7013c62..e621c5f2eeec3 100644 --- a/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php +++ b/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php @@ -72,7 +72,7 @@ public function testGroupFilters(array $expectedGroupsToRun, $group = null, $exc } // No expected lock data, means we should never call it - if (empty($expectedLockData)){ + if (empty($expectedLockData)) { $lockManager->expects($this->never()) ->method('lock'); } @@ -132,7 +132,9 @@ public function groupFiltersDataProvider(): array ['default'], // --exclude-group default ], '--exclude-group=index, all other groups should run' => [ - array_filter($listOfGroups, function($g) { return $g !== 'index'; }), // groups to run, all but index + array_filter($listOfGroups, function ($g) { + return $g !== 'index'; + }), // groups to run, all but index null, // ['index'] // --exclude-group index ], @@ -142,9 +144,13 @@ public function groupFiltersDataProvider(): array $listOfGroups // groups to exclude, all of them ], 'exclude all groups but consumers, consumers runs' => [ - array_filter($listOfGroups, function($g) { return $g === 'consumers'; }), + array_filter($listOfGroups, function ($g) { + return $g === 'consumers'; + }), null, - array_filter($listOfGroups, function($g) { return $g !== 'consumers'; }) + array_filter($listOfGroups, function ($g) { + return $g !== 'consumers'; + }) ], ]; } From def25622adb261434682f76cf20855bb353ba5af Mon Sep 17 00:00:00 2001 From: Luke Rodgers Date: Thu, 23 Sep 2021 11:29:35 +0100 Subject: [PATCH 4/8] Update test to run the same on EE and B2B Exclude the staging group etc, not relevant for our test --- .../Observer/ProcessCronQueueObserverTest.php | 56 ++++++++++++------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php b/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php index e621c5f2eeec3..93e2fb6aec701 100644 --- a/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php +++ b/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php @@ -27,18 +27,18 @@ protected function setUp(): void $this->_model->execute(new \Magento\Framework\Event\Observer()); } - /** - * @magentoConfigFixture current_store crontab/default/jobs/catalog_product_alert/schedule/cron_expr * * * * * - */ - public function testDispatchScheduled() - { - $collection = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( - \Magento\Cron\Model\ResourceModel\Schedule\Collection::class - ); - $collection->addFieldToFilter('status', \Magento\Cron\Model\Schedule::STATUS_PENDING); - $collection->addFieldToFilter('job_code', 'catalog_product_alert'); - $this->assertGreaterThan(0, $collection->count(), 'Cron has failed to schedule tasks for itself for future.'); - } +// /** +// * @magentoConfigFixture current_store crontab/default/jobs/catalog_product_alert/schedule/cron_expr * * * * * +// */ +// public function testDispatchScheduled() +// { +// $collection = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( +// \Magento\Cron\Model\ResourceModel\Schedule\Collection::class +// ); +// $collection->addFieldToFilter('status', \Magento\Cron\Model\Schedule::STATUS_PENDING); +// $collection->addFieldToFilter('job_code', 'catalog_product_alert'); +// $this->assertGreaterThan(0, $collection->count(), 'Cron has failed to schedule tasks for itself for future.'); +// } public function testDispatchNoFailed() { @@ -59,6 +59,11 @@ public function testDispatchNoFailed() */ public function testGroupFilters(array $expectedGroupsToRun, $group = null, $excludeGroup = null) { + $config = $this->createMock(\Magento\Cron\Model\ConfigInterface::class); + $config->expects($this->any()) + ->method('getJobs') + ->willReturn($this->getFilterTestCronGroups()); + $request = Bootstrap::getObjectManager()->get(\Magento\Framework\App\Console\Request::class); $lockManager = $this->createMock(\Magento\Framework\Lock\LockManagerInterface::class); @@ -91,7 +96,8 @@ public function testGroupFilters(array $expectedGroupsToRun, $group = null, $exc $this->_model = Bootstrap::getObjectManager() ->create(\Magento\Cron\Observer\ProcessCronQueueObserver::class, [ 'request' => $request, - 'lockManager' => $lockManager + 'lockManager' => $lockManager, + 'config' => $config ]); $this->_model->execute(new \Magento\Framework\Event\Observer()); } @@ -101,12 +107,7 @@ public function testGroupFilters(array $expectedGroupsToRun, $group = null, $exc */ public function groupFiltersDataProvider(): array { - $listOfGroups = []; - $config = Bootstrap::getObjectManager()->get(\Magento\Cron\Model\ConfigInterface::class); - foreach (array_keys($config->getJobs()) as $groupId) { - $listOfGroups[$groupId] = $groupId; - } - $listOfGroups = array_reverse($listOfGroups, true); + $listOfGroups = array_reverse(array_keys($this->getFilterTestCronGroups()), true); return [ 'no flags runs all groups' => [ @@ -154,4 +155,21 @@ public function groupFiltersDataProvider(): array ], ]; } + + /** + * Only run the filter group tests with a limited set of cron groups, keeps tests consistent between EE and CE + * + * @return array + */ + private function getFilterTestCronGroups() + { + $listOfGroups = []; + $config = Bootstrap::getObjectManager()->get(\Magento\Cron\Model\ConfigInterface::class); + foreach ($config->getJobs() as $groupId => $data) { + if (in_array($groupId, ['default', 'consumers', 'index'])) { + $listOfGroups[$groupId] = $data; + } + } + return $listOfGroups; + } } From fae148b2119d5593b643b4d4e01ce39a50bf5260 Mon Sep 17 00:00:00 2001 From: Luke Rodgers Date: Thu, 23 Sep 2021 11:33:26 +0100 Subject: [PATCH 5/8] Update test to run the same on EE and B2B Exclude the staging group etc, not relevant for our test --- .../Observer/ProcessCronQueueObserverTest.php | 45 ++++++++----------- 1 file changed, 19 insertions(+), 26 deletions(-) diff --git a/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php b/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php index 93e2fb6aec701..c15426adb1a9a 100644 --- a/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php +++ b/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php @@ -27,18 +27,18 @@ protected function setUp(): void $this->_model->execute(new \Magento\Framework\Event\Observer()); } -// /** -// * @magentoConfigFixture current_store crontab/default/jobs/catalog_product_alert/schedule/cron_expr * * * * * -// */ -// public function testDispatchScheduled() -// { -// $collection = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( -// \Magento\Cron\Model\ResourceModel\Schedule\Collection::class -// ); -// $collection->addFieldToFilter('status', \Magento\Cron\Model\Schedule::STATUS_PENDING); -// $collection->addFieldToFilter('job_code', 'catalog_product_alert'); -// $this->assertGreaterThan(0, $collection->count(), 'Cron has failed to schedule tasks for itself for future.'); -// } + /** + * @magentoConfigFixture current_store crontab/default/jobs/catalog_product_alert/schedule/cron_expr * * * * * + */ + public function testDispatchScheduled() + { + $collection = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( + \Magento\Cron\Model\ResourceModel\Schedule\Collection::class + ); + $collection->addFieldToFilter('status', \Magento\Cron\Model\Schedule::STATUS_PENDING); + $collection->addFieldToFilter('job_code', 'catalog_product_alert'); + $this->assertGreaterThan(0, $collection->count(), 'Cron has failed to schedule tasks for itself for future.'); + } public function testDispatchNoFailed() { @@ -107,11 +107,10 @@ public function testGroupFilters(array $expectedGroupsToRun, $group = null, $exc */ public function groupFiltersDataProvider(): array { - $listOfGroups = array_reverse(array_keys($this->getFilterTestCronGroups()), true); return [ 'no flags runs all groups' => [ - $listOfGroups // groups to run + ['consumers', 'index', 'default'] // groups to run ], '--group=default should run' => [ ['default'], // groups to run @@ -133,25 +132,19 @@ public function groupFiltersDataProvider(): array ['default'], // --exclude-group default ], '--exclude-group=index, all other groups should run' => [ - array_filter($listOfGroups, function ($g) { - return $g !== 'index'; - }), // groups to run, all but index - null, // - ['index'] // --exclude-group index + ['consumers', 'default'], // groups to run, all but index + null, // + ['index'] // --exclude-group index ], '--exclude-group for every group runs nothing' => [ [], // groups to run, none null, // - $listOfGroups // groups to exclude, all of them + ['default', 'consumers', 'index'] // groups to exclude, all of them ], 'exclude all groups but consumers, consumers runs' => [ - array_filter($listOfGroups, function ($g) { - return $g === 'consumers'; - }), + ['consumers'], null, - array_filter($listOfGroups, function ($g) { - return $g !== 'consumers'; - }) + ['index', 'default'] ], ]; } From ef36a16e4200284e621a5eea473d65b21411a29d Mon Sep 17 00:00:00 2001 From: Luke Rodgers Date: Fri, 24 Sep 2021 13:41:43 +0100 Subject: [PATCH 6/8] Update docblock in CronCommand.php https://github.com/magento/magento2/pull/34117#issuecomment-926587976 --- app/code/Magento/Cron/Console/Command/CronCommand.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Cron/Console/Command/CronCommand.php b/app/code/Magento/Cron/Console/Command/CronCommand.php index b3a5f570e939f..aad9bcf33c64d 100644 --- a/app/code/Magento/Cron/Console/Command/CronCommand.php +++ b/app/code/Magento/Cron/Console/Command/CronCommand.php @@ -64,7 +64,7 @@ public function __construct( } /** - * {@inheritdoc} + * Configures the current command. */ protected function configure() { @@ -97,7 +97,11 @@ protected function configure() /** * Runs cron jobs if cron is not disabled in Magento configurations * - * {@inheritdoc} + * @param InputInterface $input + * @param OutputInterface $output + * @return int|void + * @throws \Magento\Framework\Exception\FileSystemException + * @throws \Magento\Framework\Exception\RuntimeException */ protected function execute(InputInterface $input, OutputInterface $output) { From c6c26d30c435efddb18543430678ad8fbcc26f6e Mon Sep 17 00:00:00 2001 From: Luke Rodgers Date: Fri, 24 Sep 2021 14:31:16 +0100 Subject: [PATCH 7/8] Update docblock in CronCommand.php --- app/code/Magento/Cron/Console/Command/CronCommand.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/code/Magento/Cron/Console/Command/CronCommand.php b/app/code/Magento/Cron/Console/Command/CronCommand.php index aad9bcf33c64d..d5d9a396ccaa8 100644 --- a/app/code/Magento/Cron/Console/Command/CronCommand.php +++ b/app/code/Magento/Cron/Console/Command/CronCommand.php @@ -100,8 +100,6 @@ protected function configure() * @param InputInterface $input * @param OutputInterface $output * @return int|void - * @throws \Magento\Framework\Exception\FileSystemException - * @throws \Magento\Framework\Exception\RuntimeException */ protected function execute(InputInterface $input, OutputInterface $output) { From 34f9f725a7b85ca60f8450e4a309c8c3f1e2a11f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cengcom-Echo=E2=80=9D?= Date: Mon, 19 Dec 2022 18:11:53 +0530 Subject: [PATCH 8/8] Fixed failing integration test --- .../Magento/Cron/Observer/ProcessCronQueueObserverTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php b/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php index c15426adb1a9a..237f2c95606a8 100644 --- a/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php +++ b/dev/tests/integration/testsuite/Magento/Cron/Observer/ProcessCronQueueObserverTest.php @@ -110,7 +110,7 @@ public function groupFiltersDataProvider(): array return [ 'no flags runs all groups' => [ - ['consumers', 'index', 'default'] // groups to run + ['index', 'consumers', 'default'] // groups to run ], '--group=default should run' => [ ['default'], // groups to run