Skip to content

Commit ccc0f6c

Browse files
ishakhsuvarovishakhsuvarov
committed
MAGETWO-70195: Merge branch 'develop' of github.com:magento/magento2ce into MAGETWO-70195-PR-10047
2 parents 24135e9 + d03bc98 commit ccc0f6c

File tree

11 files changed

+307
-16
lines changed

11 files changed

+307
-16
lines changed

‎app/code/Magento/Cron/Model/ResourceModel/Schedule.php

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,10 @@ public function _construct()
2323
}
2424

2525
/**
26-
* If job is currently in $currentStatus, set it to $newStatus
27-
* and return true. Otherwise, return false and do not change the job.
28-
* This method is used to implement locking for cron jobs.
26+
* Sets new schedule status only if it's in the expected current status.
27+
*
28+
* If schedule is currently in $currentStatus, set it to $newStatus and
29+
* return true. Otherwise, return false.
2930
*
3031
* @param string $scheduleId
3132
* @param string $newStatus
@@ -45,4 +46,41 @@ public function trySetJobStatusAtomic($scheduleId, $newStatus, $currentStatus)
4546
}
4647
return false;
4748
}
49+
50+
/**
51+
* Sets schedule status only if no existing schedules with the same job code
52+
* have that status. This is used to implement locking for cron jobs.
53+
*
54+
* If the schedule is currently in $currentStatus and there are no existing
55+
* schedules with the same job code and $newStatus, set the schedule to
56+
* $newStatus and return true. Otherwise, return false.
57+
*
58+
* @param string $scheduleId
59+
* @param string $newStatus
60+
* @param string $currentStatus
61+
* @return bool
62+
*/
63+
public function trySetJobUniqueStatusAtomic($scheduleId, $newStatus, $currentStatus)
64+
{
65+
$connection = $this->getConnection();
66+
67+
$match = $connection->quoteInto('existing.job_code = current.job_code AND existing.status = ?', $newStatus);
68+
$selectIfUnlocked = $connection->select()
69+
->joinLeft(
70+
['existing' => $this->getTable('cron_schedule')],
71+
$match,
72+
['status' => new \Zend_Db_Expr($connection->quote($newStatus))]
73+
)
74+
->where('current.schedule_id = ?', $scheduleId)
75+
->where('current.status = ?', $currentStatus)
76+
->where('existing.schedule_id IS NULL');
77+
78+
$update = $connection->updateFromSelect($selectIfUnlocked, ['current' => $this->getTable('cron_schedule')]);
79+
$result = $connection->query($update)->rowCount();
80+
81+
if ($result == 1) {
82+
return true;
83+
}
84+
return false;
85+
}
4886
}

‎app/code/Magento/Cron/Model/Schedule.php

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -221,16 +221,17 @@ public function getNumeric($value)
221221
}
222222

223223
/**
224-
* Sets a job to STATUS_RUNNING only if it is currently in STATUS_PENDING.
225-
* Returns true if status was changed and false otherwise.
224+
* Lock the cron job so no other scheduled instances run simultaneously.
226225
*
227-
* This is used to implement locking for cron jobs.
226+
* Sets a job to STATUS_RUNNING only if it is currently in STATUS_PENDING
227+
* and no other jobs of the same code are currently in STATUS_RUNNING.
228+
* Returns true if status was changed and false otherwise.
228229
*
229230
* @return boolean
230231
*/
231232
public function tryLockJob()
232233
{
233-
if ($this->_getResource()->trySetJobStatusAtomic(
234+
if ($this->_getResource()->trySetJobUniqueStatusAtomic(
234235
$this->getId(),
235236
self::STATUS_RUNNING,
236237
self::STATUS_PENDING

‎app/code/Magento/Cron/Test/Unit/Model/ScheduleTest.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ protected function setUp()
2323

2424
$this->resourceJobMock = $this->getMockBuilder(\Magento\Cron\Model\ResourceModel\Schedule::class)
2525
->disableOriginalConstructor()
26-
->setMethods(['trySetJobStatusAtomic', '__wakeup', 'getIdFieldName'])
26+
->setMethods(['trySetJobUniqueStatusAtomic', '__wakeup', 'getIdFieldName'])
2727
->getMockForAbstractClass();
2828

2929
$this->resourceJobMock->expects($this->any())
@@ -336,7 +336,7 @@ public function testTryLockJobSuccess()
336336
$scheduleId = 1;
337337

338338
$this->resourceJobMock->expects($this->once())
339-
->method('trySetJobStatusAtomic')
339+
->method('trySetJobUniqueStatusAtomic')
340340
->with($scheduleId, Schedule::STATUS_RUNNING, Schedule::STATUS_PENDING)
341341
->will($this->returnValue(true));
342342

@@ -360,7 +360,7 @@ public function testTryLockJobFailure()
360360
$scheduleId = 1;
361361

362362
$this->resourceJobMock->expects($this->once())
363-
->method('trySetJobStatusAtomic')
363+
->method('trySetJobUniqueStatusAtomic')
364364
->with($scheduleId, Schedule::STATUS_RUNNING, Schedule::STATUS_PENDING)
365365
->will($this->returnValue(false));
366366

‎dev/tests/integration/testsuite/Magento/Cron/Model/ScheduleTest.php

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\Cron\Model;
7+
8+
use Magento\Framework\Stdlib\DateTime\DateTime;
9+
use \Magento\TestFramework\Helper\Bootstrap;
10+
11+
/**
12+
* Test \Magento\Cron\Model\Schedule
13+
*
14+
* @magentoDbIsolation enabled
15+
*/
16+
class ScheduleTest extends \PHPUnit_Framework_TestCase
17+
{
18+
/**
19+
* @var ScheduleFactory
20+
*/
21+
private $scheduleFactory;
22+
23+
/**
24+
* @var DateTime
25+
*/
26+
protected $dateTime;
27+
28+
public function setUp()
29+
{
30+
$this->dateTime = Bootstrap::getObjectManager()->create(DateTime::class);
31+
$this->scheduleFactory = Bootstrap::getObjectManager()->create(ScheduleFactory::class);
32+
}
33+
34+
/**
35+
* If there are no currently locked jobs, locking one of them should succeed
36+
*/
37+
public function testTryLockJobNoLockedJobsSucceeds()
38+
{
39+
for ($i = 1; $i < 6; $i++) {
40+
$this->createSchedule("test_job", Schedule::STATUS_PENDING, 60 * $i);
41+
}
42+
$schedule = $this->createSchedule("test_job", Schedule::STATUS_PENDING);
43+
44+
$this->assertTrue($schedule->tryLockJob());
45+
}
46+
47+
/**
48+
* If the job is already locked, attempting to lock it again should fail
49+
*/
50+
public function testTryLockJobAlreadyLockedFails()
51+
{
52+
$schedule = $this->createSchedule("test_job", Schedule::STATUS_RUNNING);
53+
54+
$this->assertFalse($schedule->tryLockJob());
55+
}
56+
57+
/**
58+
* If there's a job already locked, should not be able to lock another job
59+
*/
60+
public function testTryLockJobOtherLockedFails()
61+
{
62+
$this->createSchedule("test_job", Schedule::STATUS_RUNNING);
63+
$schedule = $this->createSchedule("test_job", Schedule::STATUS_PENDING, 60);
64+
65+
$this->assertFalse($schedule->tryLockJob());
66+
}
67+
68+
/**
69+
* Should be able to lock a job if a job with a different code is locked
70+
*/
71+
public function testTryLockJobDifferentJobLocked()
72+
{
73+
$this->createSchedule("test_job_other", Schedule::STATUS_RUNNING);
74+
$schedule = $this->createSchedule("test_job", Schedule::STATUS_PENDING);
75+
76+
$this->assertTrue($schedule->tryLockJob());
77+
}
78+
79+
/**
80+
* Creates a schedule with the given job code, status, and schedule time offset
81+
*
82+
* @param string $jobCode
83+
* @param string $status
84+
* @param int $timeOffset
85+
* @return Schedule
86+
*/
87+
private function createSchedule($jobCode, $status, $timeOffset = 0)
88+
{
89+
$schedule = $this->scheduleFactory->create()
90+
->setCronExpr("* * * * *")
91+
->setJobCode($jobCode)
92+
->setStatus($status)
93+
->setCreatedAt(strftime('%Y-%m-%d %H:%M:%S', $this->dateTime->gmtTimestamp()))
94+
->setScheduledAt(strftime('%Y-%m-%d %H:%M', $this->dateTime->gmtTimestamp() + $timeOffset));
95+
$schedule->save();
96+
97+
return $schedule;
98+
}
99+
}

‎dev/tests/integration/testsuite/Magento/Framework/ObjectManager/Config/Reader/DomTest.php

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@
55
*/
66
namespace Magento\Framework\ObjectManager\Config\Reader;
77

8+
/**
9+
* Class DomTest @covers \Magento\Framework\ObjectManager\Config\Reader\Dom
10+
*
11+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
12+
*/
813
class DomTest extends \PHPUnit_Framework_TestCase
914
{
1015
/**
@@ -58,8 +63,9 @@ protected function setUp()
5863
false
5964
);
6065
$this->_fileResolverMock->expects($this->once())->method('get')->will($this->returnValue($this->_fileList));
61-
$this->_mapper = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(
62-
\Magento\Framework\ObjectManager\Config\Mapper\Dom::class
66+
$this->_mapper = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create(
67+
\Magento\Framework\ObjectManager\Config\Mapper\Dom::class,
68+
['argumentInterpreter' => $this->getArgumentInterpreterWithMockedStringUtils()]
6369
);
6470
$this->_validationState = new \Magento\Framework\App\Arguments\ValidationState(
6571
\Magento\Framework\App\State::MODE_DEFAULT
@@ -80,4 +86,47 @@ public function testRead()
8086
);
8187
$this->assertEquals($this->_mapper->convert($this->_mergedConfig), $model->read('scope'));
8288
}
89+
90+
/**
91+
* Replace Magento\Framework\Data\Argument\Interpreter\StringUtils with mock to check arguments wasn't translated.
92+
*
93+
* Check argument $data has not key $data['translate'], therefore
94+
* Magento\Framework\Data\Argument\Interpreter\StringUtils::evaluate($data) won't translate $data['value'].
95+
*
96+
* @return \Magento\Framework\Data\Argument\Interpreter\Composite
97+
*/
98+
private function getArgumentInterpreterWithMockedStringUtils()
99+
{
100+
$booleanUtils = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(
101+
\Magento\Framework\Stdlib\BooleanUtils::class
102+
);
103+
$stringUtilsMock = $this->getMockBuilder(\Magento\Framework\Data\Argument\Interpreter\StringUtils::class)
104+
->setConstructorArgs(['booleanUtils' => $booleanUtils])
105+
->setMethods(['evaluate'])
106+
->getMock();
107+
$stringUtilsMock->expects($this->any())
108+
->method('evaluate')
109+
->with(self::callback(function ($data) {
110+
return !isset($data['translate']);
111+
}))
112+
->will(self::returnCallback(function ($data) {
113+
return isset($data['value']) ? $data['value'] : '';
114+
}));
115+
$constInterpreter = new \Magento\Framework\Data\Argument\Interpreter\Constant();
116+
$composite = new \Magento\Framework\Data\Argument\Interpreter\Composite(
117+
[
118+
'boolean' => new \Magento\Framework\Data\Argument\Interpreter\Boolean($booleanUtils),
119+
'string' => $stringUtilsMock,
120+
'number' => new \Magento\Framework\Data\Argument\Interpreter\Number(),
121+
'null' => new \Magento\Framework\Data\Argument\Interpreter\NullType(),
122+
'object' => new \Magento\Framework\Data\Argument\Interpreter\DataObject($booleanUtils),
123+
'const' => $constInterpreter,
124+
'init_parameter' => new \Magento\Framework\App\Arguments\ArgumentInterpreter($constInterpreter),
125+
],
126+
\Magento\Framework\ObjectManager\Config\Reader\Dom::TYPE_ATTRIBUTE
127+
);
128+
$composite->addInterpreter('array', new \Magento\Framework\Data\Argument\Interpreter\ArrayType($composite));
129+
130+
return $composite;
131+
}
83132
}

‎dev/tests/integration/testsuite/Magento/Framework/ObjectManager/_files/config_one.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
<item name="boolZero" xsi:type="boolean">false</item>
2020
<item name="intValue" xsi:type="number">100500</item>
2121
<item name="nullValue" xsi:type="null"/>
22-
<item name="stringPattern" xsi:type="string">az-value</item>
22+
<item name="stringPattern" xsi:type="string" translate="true">az-value</item>
2323
</argument>
2424
<argument name="constParam" xsi:type="const">Magento\Store\Model\Website::CACHE_TAG</argument>
2525
<argument name="boolFalseParam" xsi:type="boolean">false</argument>
@@ -28,7 +28,7 @@
2828
<argument name="boolZeroParam" xsi:type="boolean">false</argument>
2929
<argument name="intValueParam" xsi:type="number">100500</argument>
3030
<argument name="nullValueParam" xsi:type="null"/>
31-
<argument name="stringPatternParam" xsi:type="string">az-value</argument>
31+
<argument name="stringPatternParam" xsi:type="string" translate="true">az-value</argument>
3232
</arguments>
3333
</type>
3434
</config>

‎dev/tests/integration/testsuite/Magento/Framework/ObjectManager/_files/config_two.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
<item name="boolTrue" xsi:type="number">10</item>
1717
<item name="boolOne" xsi:type="string">1</item>
1818
<item name="boolZero" xsi:type="boolean">false</item>
19-
<item name="stringPattern" xsi:type="string">Az-Value</item>
19+
<item name="stringPattern" xsi:type="string" translate="true">Az-Value</item>
2020
</argument>
2121
<argument name="constParam" xsi:type="const">Magento\Store\Model\Website::CACHE_TAG</argument>
2222
<argument name="boolFalseParam" xsi:type="number">100</argument>

‎lib/internal/Magento/Framework/Config/Converter/Dom/DiFlat.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
namespace Magento\Framework\Config\Converter\Dom;
8+
9+
/**
10+
* Converter of XML data to an array representation with no data loss excluding argument translation.
11+
*/
12+
class DiFlat extends Flat
13+
{
14+
/**
15+
* Retrieve key-value pairs of node attributes excluding translate attribute.
16+
*
17+
* @param \DOMNode $node
18+
* @return array
19+
*/
20+
protected function getNodeAttributes(\DOMNode $node)
21+
{
22+
$result = parent::getNodeAttributes($node);
23+
unset($result['translate']);
24+
25+
return $result;
26+
}
27+
}

‎lib/internal/Magento/Framework/Config/Test/Unit/Converter/Dom/DiFlatTest.php

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
namespace Magento\Framework\Config\Test\Unit\Converter\Dom;
8+
9+
/**
10+
* Class DiFlatTest @covers \Magento\Framework\Config\Converter\Dom\DiFlat
11+
*/
12+
class DiFlatTest extends \PHPUnit_Framework_TestCase
13+
{
14+
/**
15+
* Test subject.
16+
*
17+
* @var \Magento\Framework\Config\Converter\Dom\DiFlat
18+
*/
19+
private $model;
20+
21+
/**
22+
* @inheritdoc
23+
*/
24+
protected function setUp()
25+
{
26+
$arrayNodeConfig = new \Magento\Framework\Config\Dom\ArrayNodeConfig(
27+
new \Magento\Framework\Config\Dom\NodePathMatcher(),
28+
[
29+
'/root/multipleNode' => 'id',
30+
'/root/wrongArray' => 'id',
31+
],
32+
[
33+
'/root/node_one/subnode',
34+
]
35+
);
36+
$this->model = new \Magento\Framework\Config\Converter\Dom\DiFlat($arrayNodeConfig);
37+
}
38+
39+
/**
40+
* Test \Magento\Framework\Config\Converter\Dom\DiFlat::convert() exclude attribute 'translate'.
41+
*
42+
* @covers \Magento\Framework\Config\Converter\Dom\DiFlat::convert()
43+
*/
44+
public function testConvert()
45+
{
46+
$fixturePath = __DIR__ . '/../../_files/converter/dom/flat/';
47+
$expected = require $fixturePath . 'result.php';
48+
49+
$dom = new \DOMDocument();
50+
$dom->load($fixturePath . 'di_source.xml');
51+
52+
$actual = $this->model->convert($dom);
53+
$this->assertEquals($expected, $actual);
54+
}
55+
}

0 commit comments

Comments
 (0)