Skip to content

Commit aa4b89c

Browse files
rostyslav-hymonrostyslav-hymon
committed
MAGETWO-97866: [Magento Cloud] news_from_date and news_to_date dates incorrect in database with scheduled updates
Due to unstable work of intl parsing on different platforms
1 parent 2957731 commit aa4b89c

File tree

2 files changed

+136
-36
lines changed

2 files changed

+136
-36
lines changed

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

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Magento\Framework\Exception\CronException;
1010
use Magento\Framework\App\ObjectManager;
1111
use Magento\Framework\Stdlib\DateTime\TimezoneInterface;
12+
use Magento\Framework\Intl\DateTimeFactory;
1213

1314
/**
1415
* Crontab schedule model
@@ -50,24 +51,32 @@ class Schedule extends \Magento\Framework\Model\AbstractModel
5051
*/
5152
private $timezoneConverter;
5253

54+
/**
55+
* @var DateTimeFactory
56+
*/
57+
private $dateTimeFactory;
58+
5359
/**
5460
* @param \Magento\Framework\Model\Context $context
5561
* @param \Magento\Framework\Registry $registry
5662
* @param \Magento\Framework\Model\ResourceModel\AbstractResource $resource
5763
* @param \Magento\Framework\Data\Collection\AbstractDb $resourceCollection
5864
* @param array $data
5965
* @param TimezoneInterface $timezoneConverter
66+
* @param DateTimeFactory $dateTimeFactory
6067
*/
6168
public function __construct(
6269
\Magento\Framework\Model\Context $context,
6370
\Magento\Framework\Registry $registry,
6471
\Magento\Framework\Model\ResourceModel\AbstractResource $resource = null,
6572
\Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null,
6673
array $data = [],
67-
TimezoneInterface $timezoneConverter = null
74+
TimezoneInterface $timezoneConverter = null,
75+
DateTimeFactory $dateTimeFactory = null
6876
) {
6977
parent::__construct($context, $registry, $resource, $resourceCollection, $data);
7078
$this->timezoneConverter = $timezoneConverter ?: ObjectManager::getInstance()->get(TimezoneInterface::class);
79+
$this->dateTimeFactory = $dateTimeFactory ?: ObjectManager::getInstance()->get(DateTimeFactory::class);
7180
}
7281

7382
/**
@@ -111,17 +120,20 @@ public function trySchedule()
111120
if (!$e || !$time) {
112121
return false;
113122
}
123+
$configTimeZone = $this->timezoneConverter->getConfigTimezone();
124+
$storeDateTime = $this->dateTimeFactory->create(null, new \DateTimeZone($configTimeZone));
114125
if (!is_numeric($time)) {
115126
//convert time from UTC to admin store timezone
116127
//we assume that all schedules in configuration (crontab.xml and DB tables) are in admin store timezone
117-
$time = $this->timezoneConverter->date($time)->format('Y-m-d H:i');
118-
$time = strtotime($time);
128+
$dateTimeUtc = $this->dateTimeFactory->create($time, new \DateTimeZone('UTC'));
129+
$time = $dateTimeUtc->getTimestamp();
119130
}
120-
$match = $this->matchCronExpression($e[0], strftime('%M', $time))
121-
&& $this->matchCronExpression($e[1], strftime('%H', $time))
122-
&& $this->matchCronExpression($e[2], strftime('%d', $time))
123-
&& $this->matchCronExpression($e[3], strftime('%m', $time))
124-
&& $this->matchCronExpression($e[4], strftime('%w', $time));
131+
$time = $storeDateTime->setTimestamp($time);
132+
$match = $this->matchCronExpression($e[0], $time->format('i'))
133+
&& $this->matchCronExpression($e[1], $time->format('H'))
134+
&& $this->matchCronExpression($e[2], $time->format('d'))
135+
&& $this->matchCronExpression($e[3], $time->format('m'))
136+
&& $this->matchCronExpression($e[4], $time->format('w'));
125137

126138
return $match;
127139
}

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

Lines changed: 116 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
namespace Magento\Cron\Test\Unit\Model;
77

88
use Magento\Cron\Model\Schedule;
9+
use Magento\Framework\Intl\DateTimeFactory;
10+
use Magento\Framework\Stdlib\DateTime\TimezoneInterface;
11+
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
912

1013
/**
1114
* Class \Magento\Cron\Test\Unit\Model\ObserverTest
@@ -18,11 +21,27 @@ class ScheduleTest extends \PHPUnit\Framework\TestCase
1821
*/
1922
protected $helper;
2023

24+
/**
25+
* @var \Magento\Cron\Model\ResourceModel\Schedule
26+
*/
2127
protected $resourceJobMock;
2228

29+
/**
30+
* @var TimezoneInterface|\PHPUnit_Framework_MockObject_MockObject
31+
*/
32+
private $timezoneConverter;
33+
34+
/**
35+
* @var DateTimeFactory|\PHPUnit_Framework_MockObject_MockObject
36+
*/
37+
private $dateTimeFactory;
38+
39+
/**
40+
* @inheritdoc
41+
*/
2342
protected function setUp()
2443
{
25-
$this->helper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
44+
$this->helper = new ObjectManager($this);
2645

2746
$this->resourceJobMock = $this->getMockBuilder(\Magento\Cron\Model\ResourceModel\Schedule::class)
2847
->disableOriginalConstructor()
@@ -32,18 +51,30 @@ protected function setUp()
3251
$this->resourceJobMock->expects($this->any())
3352
->method('getIdFieldName')
3453
->will($this->returnValue('id'));
54+
55+
$this->timezoneConverter = $this->getMockBuilder(TimezoneInterface::class)
56+
->setMethods(['date'])
57+
->getMockForAbstractClass();
58+
59+
$this->dateTimeFactory = $this->getMockBuilder(DateTimeFactory::class)
60+
->setMethods(['create'])
61+
->getMock();
3562
}
3663

3764
/**
65+
* Test for SetCronExpr
66+
*
3867
* @param string $cronExpression
3968
* @param array $expected
69+
*
70+
* @return void
4071
* @dataProvider setCronExprDataProvider
4172
*/
42-
public function testSetCronExpr($cronExpression, $expected)
73+
public function testSetCronExpr($cronExpression, $expected): void
4374
{
4475
// 1. Create mocks
45-
/** @var \Magento\Cron\Model\Schedule $model */
46-
$model = $this->helper->getObject(\Magento\Cron\Model\Schedule::class);
76+
/** @var Schedule $model */
77+
$model = $this->helper->getObject(Schedule::class);
4778

4879
// 2. Run tested method
4980
$model->setCronExpr($cronExpression);
@@ -61,7 +92,7 @@ public function testSetCronExpr($cronExpression, $expected)
6192
*
6293
* @return array
6394
*/
64-
public function setCronExprDataProvider()
95+
public function setCronExprDataProvider(): array
6596
{
6697
return [
6798
['1 2 3 4 5', [1, 2, 3, 4, 5]],
@@ -121,27 +152,33 @@ public function setCronExprDataProvider()
121152
}
122153

123154
/**
155+
* Test for SetCronExprException
156+
*
124157
* @param string $cronExpression
158+
*
159+
* @return void
125160
* @expectedException \Magento\Framework\Exception\CronException
126161
* @dataProvider setCronExprExceptionDataProvider
127162
*/
128-
public function testSetCronExprException($cronExpression)
163+
public function testSetCronExprException($cronExpression): void
129164
{
130165
// 1. Create mocks
131-
/** @var \Magento\Cron\Model\Schedule $model */
132-
$model = $this->helper->getObject(\Magento\Cron\Model\Schedule::class);
166+
/** @var Schedule $model */
167+
$model = $this->helper->getObject(Schedule::class);
133168

134169
// 2. Run tested method
135170
$model->setCronExpr($cronExpression);
136171
}
137172

138173
/**
174+
* Data provider
175+
*
139176
* Here is a list of allowed characters and values for Cron expression
140177
* http://docs.oracle.com/cd/E12058_01/doc/doc.1014/e12030/cron_expressions.htm
141178
*
142179
* @return array
143180
*/
144-
public function setCronExprExceptionDataProvider()
181+
public function setCronExprExceptionDataProvider(): array
145182
{
146183
return [
147184
[''],
@@ -153,17 +190,31 @@ public function setCronExprExceptionDataProvider()
153190
}
154191

155192
/**
193+
* Test for trySchedule
194+
*
156195
* @param int $scheduledAt
157196
* @param array $cronExprArr
158197
* @param $expected
198+
*
199+
* @return void
159200
* @dataProvider tryScheduleDataProvider
160201
*/
161-
public function testTrySchedule($scheduledAt, $cronExprArr, $expected)
202+
public function testTrySchedule($scheduledAt, $cronExprArr, $expected): void
162203
{
163204
// 1. Create mocks
205+
$this->timezoneConverter->method('getConfigTimezone')
206+
->willReturn('UTC');
207+
208+
$this->dateTimeFactory->method('create')
209+
->willReturn(new \DateTime(null, new \DateTimeZone('UTC')));
210+
164211
/** @var \Magento\Cron\Model\Schedule $model */
165212
$model = $this->helper->getObject(
166-
\Magento\Cron\Model\Schedule::class
213+
\Magento\Cron\Model\Schedule::class,
214+
[
215+
'timezoneConverter' => $this->timezoneConverter,
216+
'dateTimeFactory' => $this->dateTimeFactory
217+
]
167218
);
168219

169220
// 2. Set fixtures
@@ -177,22 +228,29 @@ public function testTrySchedule($scheduledAt, $cronExprArr, $expected)
177228
$this->assertEquals($expected, $result);
178229
}
179230

180-
public function testTryScheduleWithConversionToAdminStoreTime()
231+
/**
232+
* Test for tryScheduleWithConversionToAdminStoreTime
233+
*
234+
* @return void
235+
*/
236+
public function testTryScheduleWithConversionToAdminStoreTime(): void
181237
{
182238
$scheduledAt = '2011-12-13 14:15:16';
183239
$cronExprArr = ['*', '*', '*', '*', '*'];
184240

185-
// 1. Create mocks
186-
$timezoneConverter = $this->createMock(\Magento\Framework\Stdlib\DateTime\TimezoneInterface::class);
187-
$timezoneConverter->expects($this->once())
188-
->method('date')
189-
->with($scheduledAt)
190-
->willReturn(new \DateTime($scheduledAt));
241+
$this->timezoneConverter->method('getConfigTimezone')
242+
->willReturn('UTC');
243+
244+
$this->dateTimeFactory->method('create')
245+
->willReturn(new \DateTime(null, new \DateTimeZone('UTC')));
191246

192247
/** @var \Magento\Cron\Model\Schedule $model */
193248
$model = $this->helper->getObject(
194249
\Magento\Cron\Model\Schedule::class,
195-
['timezoneConverter' => $timezoneConverter]
250+
[
251+
'timezoneConverter' => $this->timezoneConverter,
252+
'dateTimeFactory' => $this->dateTimeFactory
253+
]
196254
);
197255

198256
// 2. Set fixtures
@@ -207,9 +265,11 @@ public function testTryScheduleWithConversionToAdminStoreTime()
207265
}
208266

209267
/**
268+
* Data provider
269+
*
210270
* @return array
211271
*/
212-
public function tryScheduleDataProvider()
272+
public function tryScheduleDataProvider(): array
213273
{
214274
$date = '2011-12-13 14:15:16';
215275
return [
@@ -229,12 +289,16 @@ public function tryScheduleDataProvider()
229289
}
230290

231291
/**
292+
* Test for matchCronExpression
293+
*
232294
* @param string $cronExpressionPart
233295
* @param int $dateTimePart
234296
* @param bool $expectedResult
297+
*
298+
* @return void
235299
* @dataProvider matchCronExpressionDataProvider
236300
*/
237-
public function testMatchCronExpression($cronExpressionPart, $dateTimePart, $expectedResult)
301+
public function testMatchCronExpression($cronExpressionPart, $dateTimePart, $expectedResult): void
238302
{
239303
// 1. Create mocks
240304
/** @var \Magento\Cron\Model\Schedule $model */
@@ -248,9 +312,11 @@ public function testMatchCronExpression($cronExpressionPart, $dateTimePart, $exp
248312
}
249313

250314
/**
315+
* Data provider
316+
*
251317
* @return array
252318
*/
253-
public function matchCronExpressionDataProvider()
319+
public function matchCronExpressionDataProvider(): array
254320
{
255321
return [
256322
['*', 0, true],
@@ -287,11 +353,15 @@ public function matchCronExpressionDataProvider()
287353
}
288354

289355
/**
356+
* Test for matchCronExpressionException
357+
*
290358
* @param string $cronExpressionPart
359+
*
360+
* @return void
291361
* @expectedException \Magento\Framework\Exception\CronException
292362
* @dataProvider matchCronExpressionExceptionDataProvider
293363
*/
294-
public function testMatchCronExpressionException($cronExpressionPart)
364+
public function testMatchCronExpressionException($cronExpressionPart): void
295365
{
296366
$dateTimePart = 10;
297367

@@ -304,9 +374,11 @@ public function testMatchCronExpressionException($cronExpressionPart)
304374
}
305375

306376
/**
377+
* Data provider
378+
*
307379
* @return array
308380
*/
309-
public function matchCronExpressionExceptionDataProvider()
381+
public function matchCronExpressionExceptionDataProvider(): array
310382
{
311383
return [
312384
['1/2/3'], //Invalid cron expression, expecting 'match/modulus': 1/2/3
@@ -317,11 +389,15 @@ public function matchCronExpressionExceptionDataProvider()
317389
}
318390

319391
/**
392+
* Test for GetNumeric
393+
*
320394
* @param mixed $param
321395
* @param int $expectedResult
396+
*
397+
* @return void
322398
* @dataProvider getNumericDataProvider
323399
*/
324-
public function testGetNumeric($param, $expectedResult)
400+
public function testGetNumeric($param, $expectedResult): void
325401
{
326402
// 1. Create mocks
327403
/** @var \Magento\Cron\Model\Schedule $model */
@@ -335,9 +411,11 @@ public function testGetNumeric($param, $expectedResult)
335411
}
336412

337413
/**
414+
* Data provider
415+
*
338416
* @return array
339417
*/
340-
public function getNumericDataProvider()
418+
public function getNumericDataProvider(): array
341419
{
342420
return [
343421
[null, false],
@@ -362,7 +440,12 @@ public function getNumericDataProvider()
362440
];
363441
}
364442

365-
public function testTryLockJobSuccess()
443+
/**
444+
* Test for tryLockJobSuccess
445+
*
446+
* @return void
447+
*/
448+
public function testTryLockJobSuccess(): void
366449
{
367450
$scheduleId = 1;
368451

@@ -386,7 +469,12 @@ public function testTryLockJobSuccess()
386469
$this->assertEquals(Schedule::STATUS_RUNNING, $model->getStatus());
387470
}
388471

389-
public function testTryLockJobFailure()
472+
/**
473+
* Test for tryLockJobFailure
474+
*
475+
* @return void
476+
*/
477+
public function testTryLockJobFailure(): void
390478
{
391479
$scheduleId = 1;
392480

0 commit comments

Comments
 (0)