Skip to content

Commit 40a433b

Browse files
committed
MC-18954: Nightly build jobs were failing lately after un-skipping tests in MC-5777
1 parent 8e199c6 commit 40a433b

File tree

3 files changed

+107
-84
lines changed

3 files changed

+107
-84
lines changed

app/code/Magento/CatalogRule/Model/Indexer/ReindexRuleProduct.php

Lines changed: 19 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,15 @@
88

99
use Magento\CatalogRule\Model\Indexer\IndexerTableSwapperInterface as TableSwapper;
1010
use Magento\Catalog\Model\ResourceModel\Indexer\ActiveTableSwitcher;
11-
use Magento\CatalogRule\Model\Rule;
12-
use Magento\Framework\App\ResourceConnection;
13-
use Magento\Framework\Stdlib\DateTime\TimezoneInterface;
14-
use Magento\Store\Model\ScopeInterface;
11+
use Magento\Framework\App\ObjectManager;
1512

1613
/**
1714
* Reindex rule relations with products.
1815
*/
1916
class ReindexRuleProduct
2017
{
2118
/**
22-
* @var ResourceConnection
19+
* @var \Magento\Framework\App\ResourceConnection
2320
*/
2421
private $resource;
2522

@@ -34,40 +31,36 @@ class ReindexRuleProduct
3431
private $tableSwapper;
3532

3633
/**
37-
* @var TimezoneInterface
38-
*/
39-
private $localeDate;
40-
41-
/**
42-
* @param ResourceConnection $resource
34+
* @param \Magento\Framework\App\ResourceConnection $resource
4335
* @param ActiveTableSwitcher $activeTableSwitcher
44-
* @param TableSwapper $tableSwapper
45-
* @param TimezoneInterface $localeDate
36+
* @param TableSwapper|null $tableSwapper
4637
*/
4738
public function __construct(
48-
ResourceConnection $resource,
39+
\Magento\Framework\App\ResourceConnection $resource,
4940
ActiveTableSwitcher $activeTableSwitcher,
50-
TableSwapper $tableSwapper,
51-
TimezoneInterface $localeDate
41+
TableSwapper $tableSwapper = null
5242
) {
5343
$this->resource = $resource;
5444
$this->activeTableSwitcher = $activeTableSwitcher;
55-
$this->tableSwapper = $tableSwapper;
56-
$this->localeDate = $localeDate;
45+
$this->tableSwapper = $tableSwapper ??
46+
ObjectManager::getInstance()->get(TableSwapper::class);
5747
}
5848

5949
/**
6050
* Reindex information about rule relations with products.
6151
*
62-
* @param Rule $rule
52+
* @param \Magento\CatalogRule\Model\Rule $rule
6353
* @param int $batchCount
6454
* @param bool $useAdditionalTable
6555
* @return bool
6656
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
6757
* @SuppressWarnings(PHPMD.NPathComplexity)
6858
*/
69-
public function execute(Rule $rule, $batchCount, $useAdditionalTable = false)
70-
{
59+
public function execute(
60+
\Magento\CatalogRule\Model\Rule $rule,
61+
$batchCount,
62+
$useAdditionalTable = false
63+
) {
7164
if (!$rule->getIsActive() || empty($rule->getWebsiteIds())) {
7265
return false;
7366
}
@@ -91,26 +84,21 @@ public function execute(Rule $rule, $batchCount, $useAdditionalTable = false)
9184

9285
$ruleId = $rule->getId();
9386
$customerGroupIds = $rule->getCustomerGroupIds();
87+
$fromTime = strtotime($rule->getFromDate());
88+
$toTime = strtotime($rule->getToDate());
89+
$toTime = $toTime ? $toTime + \Magento\CatalogRule\Model\Indexer\IndexBuilder::SECONDS_IN_DAY - 1 : 0;
9490
$sortOrder = (int)$rule->getSortOrder();
9591
$actionOperator = $rule->getSimpleAction();
9692
$actionAmount = $rule->getDiscountAmount();
9793
$actionStop = $rule->getStopRulesProcessing();
9894

9995
$rows = [];
100-
foreach ($websiteIds as $websiteId) {
101-
$scopeTz = new \DateTimeZone(
102-
$this->localeDate->getConfigTimezone(ScopeInterface::SCOPE_WEBSITE, $websiteId)
103-
);
104-
$fromTime = (new \DateTime($rule->getFromDate(), $scopeTz))->getTimestamp();
105-
$toTime = $rule->getToDate()
106-
? (new \DateTime($rule->getToDate(), $scopeTz))->getTimestamp() + IndexBuilder::SECONDS_IN_DAY - 1
107-
: 0;
10896

109-
foreach ($productIds as $productId => $validationByWebsite) {
97+
foreach ($productIds as $productId => $validationByWebsite) {
98+
foreach ($websiteIds as $websiteId) {
11099
if (empty($validationByWebsite[$websiteId])) {
111100
continue;
112101
}
113-
114102
foreach ($customerGroupIds as $customerGroupId) {
115103
$rows[] = [
116104
'rule_id' => $ruleId,
@@ -135,7 +123,6 @@ public function execute(Rule $rule, $batchCount, $useAdditionalTable = false)
135123
if (!empty($rows)) {
136124
$connection->insertMultiple($indexTable, $rows);
137125
}
138-
139126
return true;
140127
}
141128
}

app/code/Magento/CatalogRule/Model/Indexer/ReindexRuleProductPrice.php

Lines changed: 50 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -6,72 +6,82 @@
66

77
namespace Magento\CatalogRule\Model\Indexer;
88

9-
use Magento\Catalog\Model\Product;
10-
use Magento\Framework\Stdlib\DateTime\TimezoneInterface;
11-
use Magento\Store\Model\StoreManagerInterface;
12-
139
/**
1410
* Reindex product prices according rule settings.
1511
*/
1612
class ReindexRuleProductPrice
1713
{
1814
/**
19-
* @var StoreManagerInterface
15+
* @var \Magento\Store\Model\StoreManagerInterface
2016
*/
2117
private $storeManager;
2218

2319
/**
24-
* @var RuleProductsSelectBuilder
20+
* @var \Magento\CatalogRule\Model\Indexer\RuleProductsSelectBuilder
2521
*/
2622
private $ruleProductsSelectBuilder;
2723

2824
/**
29-
* @var ProductPriceCalculator
25+
* @var \Magento\CatalogRule\Model\Indexer\ProductPriceCalculator
3026
*/
3127
private $productPriceCalculator;
3228

3329
/**
34-
* @var TimezoneInterface
30+
* @var \Magento\Framework\Stdlib\DateTime\DateTime
3531
*/
36-
private $localeDate;
32+
private $dateTime;
3733

3834
/**
39-
* @var RuleProductPricesPersistor
35+
* @var \Magento\CatalogRule\Model\Indexer\RuleProductPricesPersistor
4036
*/
4137
private $pricesPersistor;
4238

4339
/**
44-
* @param StoreManagerInterface $storeManager
40+
* @var \Magento\Framework\Stdlib\DateTime\TimezoneInterface
41+
*/
42+
private $localeDate;
43+
44+
/**
45+
* @param \Magento\Store\Model\StoreManagerInterface $storeManager
4546
* @param RuleProductsSelectBuilder $ruleProductsSelectBuilder
4647
* @param ProductPriceCalculator $productPriceCalculator
47-
* @param TimezoneInterface $localeDate
48-
* @param RuleProductPricesPersistor $pricesPersistor
48+
* @param \Magento\Framework\Stdlib\DateTime\DateTime $dateTime
49+
* @param \Magento\CatalogRule\Model\Indexer\RuleProductPricesPersistor $pricesPersistor
50+
* @param \Magento\Framework\Stdlib\DateTime\TimezoneInterface $localeDate
4951
*/
5052
public function __construct(
51-
StoreManagerInterface $storeManager,
52-
RuleProductsSelectBuilder $ruleProductsSelectBuilder,
53-
ProductPriceCalculator $productPriceCalculator,
54-
TimezoneInterface $localeDate,
55-
RuleProductPricesPersistor $pricesPersistor
53+
\Magento\Store\Model\StoreManagerInterface $storeManager,
54+
\Magento\CatalogRule\Model\Indexer\RuleProductsSelectBuilder $ruleProductsSelectBuilder,
55+
\Magento\CatalogRule\Model\Indexer\ProductPriceCalculator $productPriceCalculator,
56+
\Magento\Framework\Stdlib\DateTime\DateTime $dateTime,
57+
\Magento\CatalogRule\Model\Indexer\RuleProductPricesPersistor $pricesPersistor,
58+
\Magento\Framework\Stdlib\DateTime\TimezoneInterface $localeDate
5659
) {
5760
$this->storeManager = $storeManager;
5861
$this->ruleProductsSelectBuilder = $ruleProductsSelectBuilder;
5962
$this->productPriceCalculator = $productPriceCalculator;
60-
$this->localeDate = $localeDate;
63+
$this->dateTime = $dateTime;
6164
$this->pricesPersistor = $pricesPersistor;
65+
$this->localeDate = $localeDate;
6266
}
6367

6468
/**
6569
* Reindex product prices.
6670
*
6771
* @param int $batchCount
68-
* @param Product|null $product
72+
* @param \Magento\Catalog\Model\Product|null $product
6973
* @param bool $useAdditionalTable
7074
* @return bool
7175
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
7276
*/
73-
public function execute($batchCount, Product $product = null, $useAdditionalTable = false)
74-
{
77+
public function execute(
78+
$batchCount,
79+
\Magento\Catalog\Model\Product $product = null,
80+
$useAdditionalTable = false
81+
) {
82+
$fromDate = mktime(0, 0, 0, date('m'), date('d') - 1);
83+
$toDate = mktime(0, 0, 0, date('m'), date('d') + 1);
84+
7585
/**
7686
* Update products rules prices per each website separately
7787
* because for each website date in website's timezone should be used
@@ -81,13 +91,8 @@ public function execute($batchCount, Product $product = null, $useAdditionalTabl
8191
$dayPrices = [];
8292
$stopFlags = [];
8393
$prevKey = null;
84-
8594
$storeGroup = $this->storeManager->getGroup($website->getDefaultGroupId());
86-
$currentDate = $this->localeDate->scopeDate($storeGroup->getDefaultStoreId(), null, true);
87-
$previousDate = (clone $currentDate)->modify('-1 day');
88-
$previousDate->setTime(23, 59, 59);
89-
$nextDate = (clone $currentDate)->modify('+1 day');
90-
$nextDate->setTime(0, 0, 0);
95+
$storeId = $storeGroup->getDefaultStoreId();
9196

9297
while ($ruleData = $productsStmt->fetch()) {
9398
$ruleProductId = $ruleData['product_id'];
@@ -105,24 +110,24 @@ public function execute($batchCount, Product $product = null, $useAdditionalTabl
105110
}
106111
}
107112

113+
$ruleData['from_time'] = $this->roundTime($ruleData['from_time']);
114+
$ruleData['to_time'] = $this->roundTime($ruleData['to_time']);
108115
/**
109116
* Build prices for each day
110117
*/
111-
foreach ([$previousDate, $currentDate, $nextDate] as $date) {
112-
$time = $date->getTimestamp();
118+
for ($time = $fromDate; $time <= $toDate; $time += IndexBuilder::SECONDS_IN_DAY) {
113119
if (($ruleData['from_time'] == 0 ||
114120
$time >= $ruleData['from_time']) && ($ruleData['to_time'] == 0 ||
115121
$time <= $ruleData['to_time'])
116122
) {
117123
$priceKey = $time . '_' . $productKey;
118-
119124
if (isset($stopFlags[$priceKey])) {
120125
continue;
121126
}
122127

123128
if (!isset($dayPrices[$priceKey])) {
124129
$dayPrices[$priceKey] = [
125-
'rule_date' => $date,
130+
'rule_date' => $this->localeDate->scopeDate($storeId, $time),
126131
'website_id' => $ruleData['website_id'],
127132
'customer_group_id' => $ruleData['customer_group_id'],
128133
'product_id' => $ruleProductId,
@@ -155,6 +160,19 @@ public function execute($batchCount, Product $product = null, $useAdditionalTabl
155160
}
156161
$this->pricesPersistor->execute($dayPrices, $useAdditionalTable);
157162
}
163+
158164
return true;
159165
}
166+
167+
/**
168+
* @param int $timeStamp
169+
* @return int
170+
*/
171+
private function roundTime($timeStamp)
172+
{
173+
if (is_numeric($timeStamp) && $timeStamp != 0) {
174+
$timeStamp = $this->dateTime->timestamp($this->dateTime->date('Y-m-d 00:00:00', $timeStamp));
175+
}
176+
return $timeStamp;
177+
}
160178
}

app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/ReindexRuleProductPriceTest.php

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Magento\CatalogRule\Model\Indexer\ReindexRuleProductPrice;
1212
use Magento\CatalogRule\Model\Indexer\RuleProductPricesPersistor;
1313
use Magento\CatalogRule\Model\Indexer\RuleProductsSelectBuilder;
14+
use Magento\Framework\Stdlib\DateTime\DateTime;
1415
use Magento\Framework\Stdlib\DateTime\TimezoneInterface;
1516
use Magento\Store\Api\Data\GroupInterface;
1617
use Magento\Store\Api\Data\WebsiteInterface;
@@ -40,60 +41,65 @@ class ReindexRuleProductPriceTest extends \PHPUnit\Framework\TestCase
4041
private $productPriceCalculatorMock;
4142

4243
/**
43-
* @var TimezoneInterface|MockObject
44+
* @var DateTime|MockObject
4445
*/
45-
private $localeDate;
46+
private $dateTimeMock;
4647

4748
/**
4849
* @var RuleProductPricesPersistor|MockObject
4950
*/
5051
private $pricesPersistorMock;
5152

53+
/**
54+
* @var TimezoneInterface|MockObject
55+
*/
56+
private $localeDate;
57+
5258
protected function setUp()
5359
{
5460
$this->storeManagerMock = $this->createMock(StoreManagerInterface::class);
5561
$this->ruleProductsSelectBuilderMock = $this->createMock(RuleProductsSelectBuilder::class);
5662
$this->productPriceCalculatorMock = $this->createMock(ProductPriceCalculator::class);
57-
$this->localeDate = $this->createMock(TimezoneInterface::class);
63+
$this->dateTimeMock = $this->createMock(DateTime::class);
5864
$this->pricesPersistorMock = $this->createMock(RuleProductPricesPersistor::class);
65+
$this->localeDate = $this->createMock(TimezoneInterface::class);
5966

6067
$this->model = new ReindexRuleProductPrice(
6168
$this->storeManagerMock,
6269
$this->ruleProductsSelectBuilderMock,
6370
$this->productPriceCalculatorMock,
64-
$this->localeDate,
65-
$this->pricesPersistorMock
71+
$this->dateTimeMock,
72+
$this->pricesPersistorMock,
73+
$this->localeDate
6674
);
6775
}
6876

6977
public function testExecute()
7078
{
7179
$websiteId = 234;
72-
$defaultGroupId = 11;
73-
$defaultStoreId = 22;
80+
$storeGroupId = 30;
81+
$storeId = 40;
82+
$productMock = $this->createMock(Product::class);
7483

7584
$websiteMock = $this->createMock(WebsiteInterface::class);
7685
$websiteMock->expects($this->once())
7786
->method('getId')
7887
->willReturn($websiteId);
7988
$websiteMock->expects($this->once())
8089
->method('getDefaultGroupId')
81-
->willReturn($defaultGroupId);
90+
->willReturn($storeGroupId);
8291
$this->storeManagerMock->expects($this->once())
8392
->method('getWebsites')
8493
->willReturn([$websiteMock]);
85-
$groupMock = $this->createMock(GroupInterface::class);
86-
$groupMock->method('getId')
87-
->willReturn($defaultStoreId);
88-
$groupMock->expects($this->once())
94+
$storeGroupMock = $this->createMock(GroupInterface::class);
95+
$storeGroupMock->expects($this->once())
8996
->method('getDefaultStoreId')
90-
->willReturn($defaultStoreId);
97+
->willReturn($storeId);
9198
$this->storeManagerMock->expects($this->once())
9299
->method('getGroup')
93-
->with($defaultGroupId)
94-
->willReturn($groupMock);
100+
->with($storeGroupId)
101+
->willReturn($storeGroupMock);
95102

96-
$productMock = $this->createMock(Product::class);
97103
$statementMock = $this->createMock(\Zend_Db_Statement_Interface::class);
98104
$this->ruleProductsSelectBuilderMock->expects($this->once())
99105
->method('build')
@@ -109,10 +115,22 @@ public function testExecute()
109115
'action_stop' => true
110116
];
111117

112-
$this->localeDate->expects($this->once())
113-
->method('scopeDate')
114-
->with($defaultStoreId, null, true)
115-
->willReturn(new \DateTime());
118+
$this->dateTimeMock->expects($this->at(0))
119+
->method('date')
120+
->with('Y-m-d 00:00:00', $ruleData['from_time'])
121+
->willReturn($ruleData['from_time']);
122+
$this->dateTimeMock->expects($this->at(1))
123+
->method('timestamp')
124+
->with($ruleData['from_time'])
125+
->willReturn($ruleData['from_time']);
126+
$this->dateTimeMock->expects($this->at(2))
127+
->method('date')
128+
->with('Y-m-d 00:00:00', $ruleData['to_time'])
129+
->willReturn($ruleData['to_time']);
130+
$this->dateTimeMock->expects($this->at(3))
131+
->method('timestamp')
132+
->with($ruleData['to_time'])
133+
->willReturn($ruleData['to_time']);
116134

117135
$statementMock->expects($this->at(0))
118136
->method('fetch')

0 commit comments

Comments
 (0)