Skip to content

Commit b7be473

Browse files
author
Oleksii Korshenko
authored
MAGETWO-81577: Unable to manage (install/uninstall) cron via bin/magento cron:install / cron:remove with multiple installations against same crontab #11359 
2 parents 3c59e46 + 152d323 commit b7be473

File tree

3 files changed

+64
-41
lines changed

3 files changed

+64
-41
lines changed

‎app/code/Magento/Cron/etc/di.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
</argument>
4444
</arguments>
4545
</type>
46-
<type name="Magento\Framework\Crontab\CrontabManager">
46+
<type name="Magento\Framework\Crontab\CrontabManagerInterface">
4747
<arguments>
4848
<argument name="shell" xsi:type="object">Magento\Framework\App\Shell</argument>
4949
</arguments>

‎lib/internal/Magento/Framework/Crontab/CrontabManager.php‎

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@
55
*/
66
namespace Magento\Framework\Crontab;
77

8-
use Magento\Framework\ShellInterface;
9-
use Magento\Framework\Phrase;
8+
use Magento\Framework\App\Filesystem\DirectoryList;
109
use Magento\Framework\Exception\LocalizedException;
1110
use Magento\Framework\Filesystem;
12-
use Magento\Framework\App\Filesystem\DirectoryList;
11+
use Magento\Framework\Phrase;
12+
use Magento\Framework\ShellInterface;
1313

1414
/**
1515
* Manager works with cron tasks
@@ -38,14 +38,38 @@ public function __construct(
3838
$this->filesystem = $filesystem;
3939
}
4040

41+
/**
42+
* @return string
43+
*/
44+
private function getTasksBlockStart()
45+
{
46+
$tasksBlockStart = self::TASKS_BLOCK_START;
47+
if (defined('BP')) {
48+
$tasksBlockStart .= ' ' . md5(BP);
49+
}
50+
return $tasksBlockStart;
51+
}
52+
53+
/**
54+
* @return string
55+
*/
56+
private function getTasksBlockEnd()
57+
{
58+
$tasksBlockEnd = self::TASKS_BLOCK_END;
59+
if (defined('BP')) {
60+
$tasksBlockEnd .= ' ' . md5(BP);
61+
}
62+
return $tasksBlockEnd;
63+
}
64+
4165
/**
4266
* {@inheritdoc}
4367
*/
4468
public function getTasks()
4569
{
4670
$this->checkSupportedOs();
4771
$content = $this->getCrontabContent();
48-
$pattern = '!(' . self::TASKS_BLOCK_START . ')(.*?)(' . self::TASKS_BLOCK_END . ')!s';
72+
$pattern = '!(' . $this->getTasksBlockStart() . ')(.*?)(' . $this->getTasksBlockEnd() . ')!s';
4973

5074
if (preg_match($pattern, $content, $matches)) {
5175
$tasks = trim($matches[2], PHP_EOL);
@@ -61,14 +85,14 @@ public function getTasks()
6185
*/
6286
public function saveTasks(array $tasks)
6387
{
64-
$this->checkSupportedOs();
65-
$baseDir = $this->filesystem->getDirectoryRead(DirectoryList::ROOT)->getAbsolutePath();
66-
$logDir = $this->filesystem->getDirectoryRead(DirectoryList::LOG)->getAbsolutePath();
67-
6888
if (!$tasks) {
6989
throw new LocalizedException(new Phrase('List of tasks is empty'));
7090
}
7191

92+
$this->checkSupportedOs();
93+
$baseDir = $this->filesystem->getDirectoryRead(DirectoryList::ROOT)->getAbsolutePath();
94+
$logDir = $this->filesystem->getDirectoryRead(DirectoryList::LOG)->getAbsolutePath();
95+
7296
foreach ($tasks as $key => $task) {
7397
if (empty($task['expression'])) {
7498
$tasks[$key]['expression'] = '* * * * *';
@@ -114,11 +138,11 @@ public function removeTasks()
114138
private function generateSection($content, $tasks = [])
115139
{
116140
if ($tasks) {
117-
$content .= self::TASKS_BLOCK_START . PHP_EOL;
141+
$content .= $this->getTasksBlockStart() . PHP_EOL;
118142
foreach ($tasks as $task) {
119-
$content .= $task['expression'] . ' ' . PHP_BINARY . ' '. $task['command'] . PHP_EOL;
143+
$content .= $task['expression'] . ' ' . PHP_BINARY . ' ' . $task['command'] . PHP_EOL;
120144
}
121-
$content .= self::TASKS_BLOCK_END . PHP_EOL;
145+
$content .= $this->getTasksBlockEnd() . PHP_EOL;
122146
}
123147

124148
return $content;
@@ -133,7 +157,8 @@ private function generateSection($content, $tasks = [])
133157
private function cleanMagentoSection($content)
134158
{
135159
$content = preg_replace(
136-
'!' . preg_quote(self::TASKS_BLOCK_START) . '.*?' . preg_quote(self::TASKS_BLOCK_END . PHP_EOL) . '!s',
160+
'!' . preg_quote($this->getTasksBlockStart()) . '.*?'
161+
. preg_quote($this->getTasksBlockEnd() . PHP_EOL) . '!s',
137162
'',
138163
$content
139164
);
@@ -192,7 +217,7 @@ private function checkSupportedOs()
192217
{
193218
if (stripos(PHP_OS, 'WIN') === 0) {
194219
throw new LocalizedException(
195-
new Phrase('Your operation system is not supported to work with this command')
220+
new Phrase('Your operating system is not supported to work with this command')
196221
);
197222
}
198223
}

‎lib/internal/Magento/Framework/Crontab/Test/Unit/CrontabManagerTest.php‎

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,15 @@
55
*/
66
namespace Magento\Framework\Crontab\Test\Unit;
77

8+
use Magento\Framework\App\Filesystem\DirectoryList;
89
use Magento\Framework\Crontab\CrontabManager;
910
use Magento\Framework\Crontab\CrontabManagerInterface;
10-
use Magento\Framework\ShellInterface;
11-
use Magento\Framework\Phrase;
1211
use Magento\Framework\Exception\LocalizedException;
1312
use Magento\Framework\Filesystem;
14-
use Magento\Framework\App\Filesystem\DirectoryList;
1513
use Magento\Framework\Filesystem\Directory\ReadInterface;
1614
use Magento\Framework\Filesystem\DriverPool;
15+
use Magento\Framework\Phrase;
16+
use Magento\Framework\ShellInterface;
1717

1818
class CrontabManagerTest extends \PHPUnit\Framework\TestCase
1919
{
@@ -87,17 +87,17 @@ public function getTasksDataProvider()
8787
return [
8888
[
8989
'content' => '* * * * * /bin/php /var/www/cron.php' . PHP_EOL
90-
. CrontabManagerInterface::TASKS_BLOCK_START . PHP_EOL
90+
. CrontabManagerInterface::TASKS_BLOCK_START . ' ' . md5(BP) . PHP_EOL
9191
. '* * * * * /bin/php /var/www/magento/bin/magento cron:run' . PHP_EOL
92-
. CrontabManagerInterface::TASKS_BLOCK_END . PHP_EOL,
92+
. CrontabManagerInterface::TASKS_BLOCK_END . ' ' . md5(BP) . PHP_EOL,
9393
'tasks' => ['* * * * * /bin/php /var/www/magento/bin/magento cron:run'],
9494
],
9595
[
9696
'content' => '* * * * * /bin/php /var/www/cron.php' . PHP_EOL
97-
. CrontabManagerInterface::TASKS_BLOCK_START . PHP_EOL
97+
. CrontabManagerInterface::TASKS_BLOCK_START . ' ' . md5(BP) . PHP_EOL
9898
. '* * * * * /bin/php /var/www/magento/bin/magento cron:run' . PHP_EOL
9999
. '* * * * * /bin/php /var/www/magento/bin/magento setup:cron:run' . PHP_EOL
100-
. CrontabManagerInterface::TASKS_BLOCK_END . PHP_EOL,
100+
. CrontabManagerInterface::TASKS_BLOCK_END . ' ' . md5(BP) . PHP_EOL,
101101
'tasks' => [
102102
'* * * * * /bin/php /var/www/magento/bin/magento cron:run',
103103
'* * * * * /bin/php /var/www/magento/bin/magento setup:cron:run',
@@ -165,17 +165,17 @@ public function removeTasksDataProvider()
165165
return [
166166
[
167167
'contentBefore' => '* * * * * /bin/php /var/www/cron.php' . PHP_EOL
168-
. CrontabManagerInterface::TASKS_BLOCK_START . PHP_EOL
168+
. CrontabManagerInterface::TASKS_BLOCK_START . ' ' . md5(BP) . PHP_EOL
169169
. '* * * * * /bin/php /var/www/magento/bin/magento cron:run' . PHP_EOL
170-
. CrontabManagerInterface::TASKS_BLOCK_END . PHP_EOL,
170+
. CrontabManagerInterface::TASKS_BLOCK_END . ' ' . md5(BP) . PHP_EOL,
171171
'contentAfter' => '* * * * * /bin/php /var/www/cron.php' . PHP_EOL
172172
],
173173
[
174174
'contentBefore' => '* * * * * /bin/php /var/www/cron.php' . PHP_EOL
175-
. CrontabManagerInterface::TASKS_BLOCK_START . PHP_EOL
175+
. CrontabManagerInterface::TASKS_BLOCK_START . ' ' . md5(BP) . PHP_EOL
176176
. '* * * * * /bin/php /var/www/magento/bin/magento cron:run' . PHP_EOL
177177
. '* * * * * /bin/php /var/www/magento/bin/magento setup:cron:run' . PHP_EOL
178-
. CrontabManagerInterface::TASKS_BLOCK_END . PHP_EOL,
178+
. CrontabManagerInterface::TASKS_BLOCK_END . ' ' . md5(BP) . PHP_EOL,
179179
'contentAfter' => '* * * * * /bin/php /var/www/cron.php' . PHP_EOL
180180
],
181181
[
@@ -198,14 +198,12 @@ public function testSaveTasksWithEmptyTasksList()
198198
{
199199
$baseDirMock = $this->getMockBuilder(ReadInterface::class)
200200
->getMockForAbstractClass();
201-
$baseDirMock->expects($this->once())
202-
->method('getAbsolutePath')
203-
->willReturn('/var/www/magento2/');
201+
$baseDirMock->expects($this->never())
202+
->method('getAbsolutePath');
204203
$logDirMock = $this->getMockBuilder(ReadInterface::class)
205204
->getMockForAbstractClass();
206-
$logDirMock->expects($this->once())
207-
->method('getAbsolutePath')
208-
->willReturn('/var/www/magento2/var/log/');
205+
$logDirMock->expects($this->never())
206+
->method('getAbsolutePath');
209207

210208
$this->filesystemMock->expects($this->any())
211209
->method('getDirectoryRead')
@@ -292,9 +290,9 @@ public function testSaveTasks($tasks, $content, $contentToSave)
292290
public function saveTasksDataProvider()
293291
{
294292
$content = '* * * * * /bin/php /var/www/cron.php' . PHP_EOL
295-
. CrontabManagerInterface::TASKS_BLOCK_START . PHP_EOL
293+
. CrontabManagerInterface::TASKS_BLOCK_START . ' ' . md5(BP) . PHP_EOL
296294
. '* * * * * /bin/php /var/www/magento/bin/magento cron:run' . PHP_EOL
297-
. CrontabManagerInterface::TASKS_BLOCK_END . PHP_EOL;
295+
. CrontabManagerInterface::TASKS_BLOCK_END . ' ' . md5(BP) . PHP_EOL;
298296

299297
return [
300298
[
@@ -303,41 +301,41 @@ public function saveTasksDataProvider()
303301
],
304302
'content' => $content,
305303
'contentToSave' => '* * * * * /bin/php /var/www/cron.php' . PHP_EOL
306-
. CrontabManagerInterface::TASKS_BLOCK_START . PHP_EOL
304+
. CrontabManagerInterface::TASKS_BLOCK_START . ' ' . md5(BP) . PHP_EOL
307305
. '* * * * * ' . PHP_BINARY . ' run.php' . PHP_EOL
308-
. CrontabManagerInterface::TASKS_BLOCK_END . PHP_EOL,
306+
. CrontabManagerInterface::TASKS_BLOCK_END . ' ' . md5(BP) . PHP_EOL,
309307
],
310308
[
311309
'tasks' => [
312310
['expression' => '1 2 3 4 5', 'command' => 'run.php']
313311
],
314312
'content' => $content,
315313
'contentToSave' => '* * * * * /bin/php /var/www/cron.php' . PHP_EOL
316-
. CrontabManagerInterface::TASKS_BLOCK_START . PHP_EOL
314+
. CrontabManagerInterface::TASKS_BLOCK_START . ' ' . md5(BP) . PHP_EOL
317315
. '1 2 3 4 5 ' . PHP_BINARY . ' run.php' . PHP_EOL
318-
. CrontabManagerInterface::TASKS_BLOCK_END . PHP_EOL,
316+
. CrontabManagerInterface::TASKS_BLOCK_END . ' ' . md5(BP) . PHP_EOL,
319317
],
320318
[
321319
'tasks' => [
322320
['command' => '{magentoRoot}run.php >> {magentoLog}cron.log']
323321
],
324322
'content' => $content,
325323
'contentToSave' => '* * * * * /bin/php /var/www/cron.php' . PHP_EOL
326-
. CrontabManagerInterface::TASKS_BLOCK_START . PHP_EOL
324+
. CrontabManagerInterface::TASKS_BLOCK_START . ' ' . md5(BP) . PHP_EOL
327325
. '* * * * * ' . PHP_BINARY . ' /var/www/magento2/run.php >>'
328326
. ' /var/www/magento2/var/log/cron.log' . PHP_EOL
329-
. CrontabManagerInterface::TASKS_BLOCK_END . PHP_EOL,
327+
. CrontabManagerInterface::TASKS_BLOCK_END . ' ' . md5(BP) . PHP_EOL,
330328
],
331329
[
332330
'tasks' => [
333331
['command' => '{magentoRoot}run.php % cron:run | grep -v "Ran \'jobs\' by schedule"']
334332
],
335333
'content' => $content,
336334
'contentToSave' => '* * * * * /bin/php /var/www/cron.php' . PHP_EOL
337-
. CrontabManagerInterface::TASKS_BLOCK_START . PHP_EOL
335+
. CrontabManagerInterface::TASKS_BLOCK_START . ' ' . md5(BP) . PHP_EOL
338336
. '* * * * * ' . PHP_BINARY . ' /var/www/magento2/run.php'
339337
. ' %% cron:run | grep -v \"Ran \'jobs\' by schedule\"' . PHP_EOL
340-
. CrontabManagerInterface::TASKS_BLOCK_END . PHP_EOL,
338+
. CrontabManagerInterface::TASKS_BLOCK_END . ' ' . md5(BP) . PHP_EOL,
341339
],
342340
];
343341
}

0 commit comments

Comments
 (0)