Skip to content

Commit 27a76b6

Browse files
author
Joan He
committed
Merge remote-tracking branch 'origin/MAGETWO-65185-improve-upgrade' into MAGETWO-65623-n-MAGETWO-65185
# Conflicts: # lib/internal/Magento/Framework/DB/FieldDataConverter.php
2 parents 7408861 + dc5145c commit 27a76b6

File tree

6 files changed

+233
-66
lines changed

6 files changed

+233
-66
lines changed

‎app/code/Magento/Quote/Setup/ConvertSerializedDataToJson.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ public function convert()
7272
'payment_id',
7373
'additional_information'
7474
);
75+
$fieldDataConverter->convert(
76+
$this->quoteSetup->getConnection(),
77+
$this->quoteSetup->getTable('quote_payment'),
78+
'payment_id',
79+
'additional_data'
80+
);
7581
$fieldDataConverter->convert(
7682
$this->quoteSetup->getConnection(),
7783
$this->quoteSetup->getTable('quote_address'),

‎app/etc/di.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,6 +1270,11 @@
12701270
</argument>
12711271
</arguments>
12721272
</type>
1273+
<type name="Magento\Framework\DB\FieldDataConverter">
1274+
<arguments>
1275+
<argument name="envBatchSize" xsi:type="init_parameter">Magento\Framework\DB\FieldDataConverter::BATCH_SIZE_VARIABLE_NAME</argument>
1276+
</arguments>
1277+
</type>
12731278
<type name="Magento\Framework\View\Asset\PreProcessor\Chain">
12741279
<arguments>
12751280
<argument name="compatibleTypes" xsi:type="array">

‎dev/tests/integration/testsuite/Magento/Framework/DB/DataConverter/DataConverterTest.php

Lines changed: 33 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,15 @@
88
use Magento\Framework\DB\Adapter\Pdo\Mysql;
99
use Magento\Framework\DB\FieldDataConverter;
1010
use Magento\Framework\DB\Select;
11+
use Magento\Framework\DB\Select\QueryModifierInterface;
1112
use Magento\Framework\DB\Select\InQueryModifier;
12-
use Magento\Framework\Serialize\Serializer\Serialize;
1313
use Magento\TestFramework\Helper\Bootstrap;
14+
use Magento\Framework\DB\Query\Generator;
15+
use Magento\Framework\DB\Query\BatchIterator;
16+
use Magento\Framework\ObjectManagerInterface;
1417

1518
class DataConverterTest extends \PHPUnit_Framework_TestCase
1619
{
17-
/**
18-
* @var \Magento\Framework\ObjectManagerInterface
19-
*/
20-
private $objectManager;
21-
2220
/**
2321
* @var InQueryModifier|\PHPUnit_Framework_MockObject_MockObject
2422
*/
@@ -30,12 +28,12 @@ class DataConverterTest extends \PHPUnit_Framework_TestCase
3028
private $dataConverter;
3129

3230
/**
33-
* @var \Magento\Framework\DB\Query\BatchIterator|\PHPUnit_Framework_MockObject_MockObject
31+
* @var BatchIterator|\PHPUnit_Framework_MockObject_MockObject
3432
*/
3533
private $iteratorMock;
3634

3735
/**
38-
* @var \Magento\Framework\DB\Query\Generator|\PHPUnit_Framework_MockObject_MockObject
36+
* @var Generator|\PHPUnit_Framework_MockObject_MockObject
3937
*/
4038
private $queryGeneratorMock;
4139

@@ -54,6 +52,11 @@ class DataConverterTest extends \PHPUnit_Framework_TestCase
5452
*/
5553
private $fieldDataConverter;
5654

55+
/**
56+
* @var ObjectManagerInterface
57+
*/
58+
private $objectManager;
59+
5760
/**
5861
* Set up before test
5962
*/
@@ -62,23 +65,24 @@ protected function setUp()
6265
$this->objectManager = Bootstrap::getObjectManager();
6366

6467
/** @var InQueryModifier $queryModifier */
65-
$this->queryModifierMock = $this->getMockBuilder(Select\QueryModifierInterface::class)
68+
$this->queryModifierMock = $this->getMockBuilder(QueryModifierInterface::class)
6669
->disableOriginalConstructor()
6770
->setMethods(['modify'])
6871
->getMock();
6972

7073
$this->dataConverter = $this->objectManager->get(SerializedToJson::class);
7174

72-
$this->iteratorMock = $this->getMockBuilder(\Magento\Framework\DB\Query\BatchIterator::class)
75+
$this->iteratorMock = $this->getMockBuilder(BatchIterator::class)
7376
->disableOriginalConstructor()
7477
->setMethods(['current', 'valid', 'next'])
7578
->getMock();
7679

7780
$iterationComplete = false;
7881

7982
// mock valid() call so iterator passes only current() result in foreach invocation
80-
$this->iteratorMock->expects($this->any())->method('valid')->will(
81-
$this->returnCallback(
83+
$this->iteratorMock->expects($this->any())
84+
->method('valid')
85+
->willReturnCallback(
8286
function () use (&$iterationComplete) {
8387
if (!$iterationComplete) {
8488
$iterationComplete = true;
@@ -87,10 +91,9 @@ function () use (&$iterationComplete) {
8791
return false;
8892
}
8993
}
90-
)
91-
);
94+
);
9295

93-
$this->queryGeneratorMock = $this->getMockBuilder(\Magento\Framework\DB\Query\Generator::class)
96+
$this->queryGeneratorMock = $this->getMockBuilder(Generator::class)
9497
->disableOriginalConstructor()
9598
->setMethods(['generate'])
9699
->getMock();
@@ -111,7 +114,7 @@ function () use (&$iterationComplete) {
111114

112115
$this->adapterMock = $this->getMockBuilder(Mysql::class)
113116
->disableOriginalConstructor()
114-
->setMethods(['fetchAll', 'quoteInto', 'update'])
117+
->setMethods(['fetchPairs', 'quoteInto', 'update'])
115118
->getMock();
116119

117120
$this->adapterMock->expects($this->any())
@@ -129,55 +132,49 @@ function () use (&$iterationComplete) {
129132

130133
/**
131134
* Test that exception with valid text is thrown when data is corrupted
135+
*
132136
* @expectedException \Magento\Framework\DB\FieldDataConversionException
133137
* @expectedExceptionMessage Error converting field `value` in table `table` where `id`=2 using
134138
*/
135139
public function testDataConvertErrorReporting()
136140
{
137-
/** @var Serialize $serializer */
138-
$serializer = $this->objectManager->create(Serialize::class);
139-
$serializedData = $serializer->serialize(['some' => 'data', 'other' => 'other data']);
140-
$serializedDataLength = strlen($serializedData);
141-
$brokenSerializedData = substr($serializedData, 0, $serializedDataLength - 6);
142141
$rows = [
143-
['id' => 1, 'value' => 'N;'],
144-
['id' => 2, 'value' => $brokenSerializedData],
142+
1 => 'N;',
143+
2 => 'a:2:{s:3:"foo";s:3:"bar";s:3:"bar";s:',
145144
];
146145

147146
$this->adapterMock->expects($this->any())
148-
->method('fetchAll')
147+
->method('fetchPairs')
149148
->with($this->selectByRangeMock)
150149
->will($this->returnValue($rows));
151150

152151
$this->adapterMock->expects($this->once())
153152
->method('update')
154-
->with('table', ['value' => 'null'], ['id = ?' => 1]);
153+
->with('table', ['value' => 'null'], ['id IN (?)' => [1]]);
155154

156155
$this->fieldDataConverter->convert($this->adapterMock, 'table', 'id', 'value', $this->queryModifierMock);
157156
}
158157

159-
/**
160-
*/
161158
public function testAlreadyConvertedDataSkipped()
162159
{
163160
$rows = [
164-
['id' => 2, 'value' => '[]'],
165-
['id' => 3, 'value' => '{}'],
166-
['id' => 4, 'value' => 'null'],
167-
['id' => 5, 'value' => '""'],
168-
['id' => 6, 'value' => '0'],
169-
['id' => 7, 'value' => 'N;'],
170-
['id' => 8, 'value' => '{"valid": "json value"}'],
161+
2 => '[]',
162+
3 => '{}',
163+
4 => 'null',
164+
5 => '""',
165+
6 => '0',
166+
7 => 'N;',
167+
8 => '{"valid": "json value"}',
171168
];
172169

173170
$this->adapterMock->expects($this->any())
174-
->method('fetchAll')
171+
->method('fetchPairs')
175172
->with($this->selectByRangeMock)
176173
->will($this->returnValue($rows));
177174

178175
$this->adapterMock->expects($this->once())
179176
->method('update')
180-
->with('table', ['value' => 'null'], ['id = ?' => 7]);
177+
->with('table', ['value' => 'null'], ['id IN (?)' => [7]]);
181178

182179
$this->fieldDataConverter->convert($this->adapterMock, 'table', 'id', 'value', $this->queryModifierMock);
183180
}

‎lib/internal/Magento/Framework/DB/FieldDataConverter.php

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
*/
66
namespace Magento\Framework\DB;
77

8-
use Magento\Framework\App\ObjectManager;
98
use Magento\Framework\DB\Adapter\AdapterInterface;
109
use Magento\Framework\DB\DataConverter\DataConversionException;
1110
use Magento\Framework\DB\Query\Generator;
@@ -17,6 +16,16 @@
1716
*/
1817
class FieldDataConverter
1918
{
19+
/**
20+
* Batch size env variable name
21+
*/
22+
const BATCH_SIZE_VARIABLE_NAME = 'DATA_CONVERTER_BATCH_SIZE';
23+
24+
/**
25+
* Default batch size
26+
*/
27+
const DEFAULT_BATCH_SIZE = 50000;
28+
2029
/**
2130
* @var Generator
2231
*/
@@ -32,25 +41,33 @@ class FieldDataConverter
3241
*/
3342
private $selectFactory;
3443

44+
/**
45+
* @var string|null
46+
*/
47+
private $envBatchSize;
48+
3549
/**
3650
* Constructor
3751
*
3852
* @param Generator $queryGenerator
3953
* @param DataConverterInterface $dataConverter
4054
* @param SelectFactory $selectFactory
55+
* @param string|null $envBatchSize
4156
*/
4257
public function __construct(
4358
Generator $queryGenerator,
4459
DataConverterInterface $dataConverter,
45-
SelectFactory $selectFactory
60+
SelectFactory $selectFactory,
61+
$envBatchSize = null
4662
) {
4763
$this->queryGenerator = $queryGenerator;
4864
$this->dataConverter = $dataConverter;
4965
$this->selectFactory = $selectFactory;
66+
$this->envBatchSize = $envBatchSize;
5067
}
5168

5269
/**
53-
* Convert table field data from one representation to another uses DataConverterInterface
70+
* Convert table field data from one representation to another
5471
*
5572
* @param AdapterInterface $connection
5673
* @param string $table
@@ -73,18 +90,20 @@ public function convert(
7390
if ($queryModifier) {
7491
$queryModifier->modify($select);
7592
}
76-
$iterator = $this->queryGenerator->generate($identifier, $select);
93+
$iterator = $this->queryGenerator->generate($identifier, $select, $this->getBatchSize());
7794
foreach ($iterator as $selectByRange) {
78-
$rows = $connection->fetchAll($selectByRange);
79-
foreach ($rows as $row) {
95+
$rows = $connection->fetchPairs($selectByRange);
96+
$uniqueFieldDataArray = array_unique($rows);
97+
foreach ($uniqueFieldDataArray as $uniqueFieldData) {
98+
$ids = array_keys($rows, $uniqueFieldData);
8099
try {
81-
$convertedValue = $this->dataConverter->convert($row[$field]);
82-
if ($row[$field] === $convertedValue) {
83-
// skip for data rows that have been already converted
100+
$convertedValue = $this->dataConverter->convert($uniqueFieldData);
101+
if ($uniqueFieldData === $convertedValue) {
102+
// Skip for data rows that have been already converted
84103
continue;
85104
}
86105
$bind = [$field => $convertedValue];
87-
$where = [$identifier . ' = ?' => (int) $row[$identifier]];
106+
$where = [$identifier . ' IN (?)' => $ids];
88107
$connection->update($table, $bind, $where);
89108
} catch (DataConversionException $e) {
90109
throw new \Magento\Framework\DB\FieldDataConversionException(
@@ -93,7 +112,7 @@ public function convert(
93112
$field,
94113
$table,
95114
$identifier,
96-
$row[$identifier],
115+
implode(', ', $ids),
97116
get_class($this->dataConverter),
98117
$e->getMessage()
99118
)
@@ -102,4 +121,24 @@ public function convert(
102121
}
103122
}
104123
}
124+
125+
/**
126+
* Get batch size from environment variable or default
127+
*
128+
* @return int
129+
*/
130+
private function getBatchSize()
131+
{
132+
if (null !== $this->envBatchSize) {
133+
$batchSize = (int) $this->envBatchSize;
134+
if (bccomp($this->envBatchSize, PHP_INT_MAX, 0) === 1 || $batchSize < 1) {
135+
throw new \InvalidArgumentException(
136+
'Invalid value for environment variable ' . self::BATCH_SIZE_VARIABLE_NAME . '. '
137+
. 'Should be integer, >= 1 and < value of PHP_INT_MAX'
138+
);
139+
}
140+
return $batchSize;
141+
}
142+
return self::DEFAULT_BATCH_SIZE;
143+
}
105144
}

0 commit comments

Comments
 (0)