Skip to content

Commit 8ff1c16

Browse files
committed
Merge remote-tracking branch '33161/feature/remove-unreferenced-cron-jobs' into comm_prs_248beta1
2 parents 32b9733 + 9ebddc6 commit 8ff1c16

File tree

5 files changed

+249
-3
lines changed

5 files changed

+249
-3
lines changed
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
<?php
2+
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
8+
declare(strict_types=1);
9+
10+
namespace Magento\Cron\Cron;
11+
12+
use Magento\Config\App\Config\Type\System;
13+
use Magento\Cron\Model\DeadlockRetrierInterface;
14+
use Magento\Cron\Model\ResourceModel\Schedule;
15+
use Magento\Cron\Model\ScheduleFactory;
16+
use Magento\Cron\Observer\ProcessCronQueueObserver;
17+
use Magento\Framework\App\Config;
18+
use Magento\Framework\Stdlib\DateTime\DateTime;
19+
20+
class CleanOldJobs
21+
{
22+
/**
23+
* This docblock provides no new information
24+
*
25+
* @param Config $config
26+
* @param DateTime $dateTime
27+
* @param DeadlockRetrierInterface $retrier
28+
* @param ScheduleFactory $scheduleFactory
29+
*/
30+
public function __construct(
31+
private readonly Config $config,
32+
private readonly DateTime $dateTime,
33+
private readonly DeadlockRetrierInterface $retrier,
34+
private readonly ScheduleFactory $scheduleFactory,
35+
) {
36+
}
37+
38+
/**
39+
* Run the 'clean_cron_schedule' cronjob
40+
*
41+
* @return void
42+
*/
43+
public function execute(): void
44+
{
45+
$fullConfig = $this->config->get(System::CONFIG_TYPE);
46+
$maxLifetime = 0;
47+
48+
array_walk_recursive(
49+
$fullConfig,
50+
static function ($value, $key) use (&$maxLifetime) {
51+
if ($key === ProcessCronQueueObserver::XML_PATH_HISTORY_SUCCESS
52+
|| $key === ProcessCronQueueObserver::XML_PATH_HISTORY_FAILURE
53+
) {
54+
$maxLifetime = max($maxLifetime, (int) $value);
55+
}
56+
}
57+
);
58+
59+
if ($maxLifetime === 0) {
60+
// Something has gone wrong. Why are there no configuration values?
61+
// Drop out now to avoid doing any damage to this already-broken installation.
62+
return;
63+
}
64+
65+
// The value stored in XML is in minutes, we want seconds.
66+
$maxLifetime *= 60;
67+
68+
// Add one day to avoid removing items which are near their natural expiry anyway.
69+
$maxLifetime += 86400;
70+
71+
/** @var Schedule $scheduleResource */
72+
$scheduleResource = $this->scheduleFactory->create()->getResource();
73+
74+
$currentTime = $this->dateTime->gmtTimestamp();
75+
$deleteBefore = $scheduleResource->getConnection()->formatDate($currentTime - $maxLifetime);
76+
77+
$this->retrier->execute(
78+
function () use ($scheduleResource, $deleteBefore) {
79+
$scheduleResource->getConnection()->delete(
80+
$scheduleResource->getTable('cron_schedule'),
81+
[
82+
'scheduled_at < ?' => $deleteBefore,
83+
]
84+
);
85+
},
86+
$scheduleResource->getConnection()
87+
);
88+
}
89+
}
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
<?php
2+
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
8+
declare(strict_types=1);
9+
10+
namespace Magento\Cron\Test\Unit\Cron;
11+
12+
use Magento\Cron\Cron\CleanOldJobs;
13+
use Magento\Cron\Model\DeadlockRetrierInterface;
14+
use Magento\Cron\Model\ResourceModel\Schedule as ScheduleResourceModel;
15+
use Magento\Cron\Model\Schedule;
16+
use Magento\Cron\Model\ScheduleFactory;
17+
use Magento\Framework\App\Config;
18+
use Magento\Framework\DB\Adapter\AdapterInterface;
19+
use Magento\Framework\Stdlib\DateTime\DateTime;
20+
use PHPUnit\Framework\TestCase;
21+
22+
// phpcs:disable Magento2.Commenting.ClassPropertyPHPDocFormatting.Missing, because such comments would add no value.
23+
24+
class CleanOldJobsTest extends TestCase
25+
{
26+
private CleanOldJobs $cleanOldJobs;
27+
private Config $configMock;
28+
private ScheduleFactory $scheduleFactoryMock;
29+
private DateTime $dateTimeMock;
30+
private ScheduleResourceModel $scheduleResourceMock;
31+
private DeadlockRetrierInterface $retrierMock;
32+
private Schedule $scheduleMock;
33+
private int $time = 1501538400;
34+
35+
/**
36+
* @inheritdoc
37+
*/
38+
protected function setUp(): void
39+
{
40+
$this->configMock = $this->getMockBuilder(Config::class)
41+
->disableOriginalConstructor()
42+
->getMock();
43+
44+
$this->dateTimeMock = $this->getMockBuilder(DateTime::class)
45+
->disableOriginalConstructor()
46+
->getMock();
47+
$this->dateTimeMock->method('gmtTimestamp')
48+
->willReturn($this->time);
49+
50+
$this->retrierMock = $this->getMockForAbstractClass(DeadlockRetrierInterface::class);
51+
52+
$this->scheduleFactoryMock = $this->getMockBuilder(ScheduleFactory::class)
53+
->onlyMethods(['create'])
54+
->disableOriginalConstructor()
55+
->getMock();
56+
57+
$this->scheduleMock = $this->getMockBuilder(Schedule::class)
58+
->disableOriginalConstructor()
59+
->getMock();
60+
61+
$this->scheduleResourceMock = $this->getMockBuilder(ScheduleResourceModel::class)
62+
->disableOriginalConstructor()
63+
->getMock();
64+
65+
$this->scheduleMock
66+
->method('getResource')
67+
->willReturn($this->scheduleResourceMock);
68+
69+
$this->scheduleFactoryMock
70+
->method('create')
71+
->willReturn($this->scheduleMock);
72+
73+
$this->cleanOldJobs = new CleanOldJobs(
74+
$this->configMock,
75+
$this->dateTimeMock,
76+
$this->retrierMock,
77+
$this->scheduleFactoryMock
78+
);
79+
}
80+
81+
public function testSuccess(): void
82+
{
83+
$tableName = 'cron_schedule';
84+
85+
$this->configMock->expects($this->once())
86+
->method('get')
87+
->with('system')
88+
->willReturn([
89+
'history_success_lifetime' => 100,
90+
'history_failure_lifetime' => 200,
91+
]);
92+
93+
$connectionMock = $this->getMockForAbstractClass(AdapterInterface::class);
94+
95+
$connectionMock->expects($this->once())
96+
->method('delete')
97+
->with($tableName, [
98+
'scheduled_at < ?' => '$this->time - (86400 + (200 * 60))',
99+
]);
100+
101+
$connectionMock->method('formatDate')
102+
->willReturnMap([
103+
[1501538400, true, '$this->time'],
104+
[1501538200, true, '$this->time - 200'],
105+
[1501526400, true, '$this->time - (200 * 60)'],
106+
[1501452000, true, '$this->time - 86400'],
107+
[1501451800, true, '$this->time - (86400 + 200)'],
108+
[1501440000, true, '$this->time - (86400 + (200 * 60))'],
109+
]);
110+
111+
$this->scheduleResourceMock->expects($this->once())
112+
->method('getTable')
113+
->with($tableName)
114+
->willReturn($tableName);
115+
$this->scheduleResourceMock->expects($this->exactly(3))
116+
->method('getConnection')
117+
->willReturn($connectionMock);
118+
119+
$this->retrierMock->expects($this->once())
120+
->method('execute')
121+
->willReturnCallback(
122+
function ($callback) {
123+
return $callback();
124+
}
125+
);
126+
127+
$this->cleanOldJobs->execute();
128+
}
129+
130+
public function testNoActionWhenEmptyConfig(): void
131+
{
132+
$this->configMock->expects($this->once())
133+
->method('get')
134+
->with('system')
135+
->willReturn([]);
136+
137+
$this->scheduleFactoryMock->expects($this->never())->method('create');
138+
139+
$this->cleanOldJobs->execute();
140+
}
141+
}

app/code/Magento/Cron/composer.json

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,9 @@
77
"require": {
88
"php": "~8.1.0||~8.2.0||~8.3.0",
99
"magento/framework": "*",
10+
"magento/module-config": "^100.1.2 || ^101.0",
1011
"magento/module-store": "*"
1112
},
12-
"suggest": {
13-
"magento/module-config": "*"
14-
},
1513
"type": "magento2-module",
1614
"license": [
1715
"OSL-3.0",

app/code/Magento/Cron/etc/crontab.xml

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?xml version="1.0"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
9+
xsi:noNamespaceSchemaLocation="urn:magento:module:Magento_Cron:etc/crontab.xsd">
10+
<group id="default">
11+
<job name="clean_cron_schedule"
12+
instance="Magento\Cron\Cron\CleanOldJobs"
13+
method="execute">
14+
<schedule>0 0 * * *</schedule>
15+
</job>
16+
</group>
17+
</config>

app/code/Magento/Cron/etc/module.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd">
99
<module name="Magento_Cron" >
1010
<sequence>
11+
<module name="Magento_Config"/>
1112
<module name="Magento_Store"/>
1213
</sequence>
1314
</module>

0 commit comments

Comments
 (0)