Skip to content

Commit a6b6599

Browse files
author
Alexander Akimov
authored
Merge pull request #888 from magento-troll/MAGETWO-62168
[Troll]: Magetwo 62168
2 parents 317392f + 441a95d commit a6b6599

File tree

9 files changed

+360
-47
lines changed

9 files changed

+360
-47
lines changed

app/code/Magento/Sales/Setup/SerializedDataConverter.php

Lines changed: 13 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -6,65 +6,40 @@
66

77
namespace Magento\Sales\Setup;
88

9-
use Magento\Framework\Serialize\Serializer\Serialize;
10-
use Magento\Framework\Serialize\Serializer\Json;
9+
use Magento\Framework\DB\DataConverter\DataConversionException;
10+
use Magento\Framework\DB\DataConverter\SerializedToJson;
1111

1212
/**
1313
* Serializer used to update nested serialized data in product_options field.
1414
*/
15-
class SerializedDataConverter implements \Magento\Framework\DB\DataConverter\DataConverterInterface
15+
class SerializedDataConverter extends SerializedToJson
1616
{
17-
/**
18-
* @var Serialize
19-
*/
20-
private $serialize;
21-
22-
/**
23-
* @var Json
24-
*/
25-
private $json;
26-
27-
/**
28-
* SerializedDataConverter constructor.
29-
*
30-
* @param Serialize $serialize
31-
* @param Json $json
32-
*/
33-
public function __construct(
34-
Serialize $serialize,
35-
Json $json
36-
) {
37-
$this->serialize = $serialize;
38-
$this->json = $json;
39-
}
40-
4117
/**
4218
* Convert from serialized to JSON format.
4319
*
4420
* @param string $value
4521
* @return string
22+
*
23+
* @throws DataConversionException
4624
*/
4725
public function convert($value)
4826
{
49-
$valueUnserialized = $this->serialize->unserialize($value);
27+
if ($this->isValidJsonValue($value)) {
28+
return $value;
29+
}
30+
$valueUnserialized = $this->unserializeValue($value);
5031
if (isset($valueUnserialized['options'])) {
5132
foreach ($valueUnserialized['options'] as $key => $option) {
5233
if ($option['option_type'] === 'file') {
53-
$valueUnserialized['options'][$key]['option_value'] = $this->json->serialize(
54-
$this->serialize->unserialize(
55-
$option['option_value']
56-
)
57-
);
34+
$valueUnserialized['options'][$key]['option_value'] = parent::convert($option['option_value']);
5835
}
5936
}
6037
}
6138
if (isset($valueUnserialized['bundle_selection_attributes'])) {
62-
$valueUnserialized['bundle_selection_attributes'] = $this->json->serialize(
63-
$this->serialize->unserialize(
64-
$valueUnserialized['bundle_selection_attributes']
65-
)
39+
$valueUnserialized['bundle_selection_attributes'] = parent::convert(
40+
$valueUnserialized['bundle_selection_attributes']
6641
);
6742
}
68-
return $this->json->serialize($valueUnserialized);
43+
return $this->encodeJson($valueUnserialized);
6944
}
7045
}

app/code/Magento/Sales/Test/Unit/Setup/SerializedDataConverterTest.php

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,15 @@ class SerializedDataConverterTest extends \PHPUnit_Framework_TestCase
3131
public function setUp()
3232
{
3333
$this->serializeMock = $this->getMock(Serialize::class, ['unserialize'], [], '', false);
34+
$this->jsonMock = $this->getMock(Json::class, ['serialize'], [], '', false);
35+
$this->model = new \Magento\Sales\Setup\SerializedDataConverter($this->serializeMock, $this->jsonMock);
36+
}
37+
38+
/**
39+
* Init serializer mock with default serialize and unserialize callbacks
40+
*/
41+
protected function initSerializerMock()
42+
{
3443
$this->serializeMock->expects($this->any())
3544
->method('unserialize')
3645
->will(
@@ -40,7 +49,6 @@ function ($value) {
4049
}
4150
)
4251
);
43-
$this->jsonMock = $this->getMock(Json::class, ['serialize'], [], '', false);
4452
$this->jsonMock->expects($this->any())
4553
->method('serialize')
4654
->will(
@@ -50,9 +58,6 @@ function ($value) {
5058
}
5159
)
5260
);
53-
54-
$this->model = new \Magento\Sales\Setup\SerializedDataConverter($this->serializeMock, $this->jsonMock);
55-
5661
}
5762

5863
/**
@@ -63,9 +68,37 @@ function ($value) {
6368
*/
6469
public function testConvert($serialized, $expectedJson)
6570
{
71+
$this->initSerializerMock();
6672
$this->assertEquals($expectedJson, $this->model->convert($serialized));
6773
}
6874

75+
/**
76+
* @param string $serialized
77+
* @param string $expectedJson
78+
*
79+
* @dataProvider convertDataProvider
80+
*
81+
* @expectedException \Magento\Framework\DB\DataConverter\DataConversionException
82+
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
83+
*/
84+
public function testConvertCorruptedData($serialized, $expectedJson)
85+
{
86+
$this->initSerializerMock();
87+
$serialized = substr($serialized, 0, 50);
88+
$this->model->convert($serialized);
89+
}
90+
91+
/**
92+
* Test skipping deserialization and json_encoding of valid JSON encoded string
93+
*/
94+
public function testSkipJsonDataConversion()
95+
{
96+
$serialized = '[]';
97+
$this->serializeMock->expects($this->never())->method('unserialize');
98+
$this->jsonMock->expects($this->never())->method('serialize');
99+
$this->model->convert($serialized);
100+
}
101+
69102
/**
70103
* Data provider for convert method test
71104
*
Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
<?php
2+
/**
3+
* Copyright © 2013-2017 Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\Framework\DB\DataConverter;
7+
8+
use Magento\Framework\DB\Adapter\Pdo\Mysql;
9+
use Magento\Framework\DB\FieldDataConverter;
10+
use Magento\Framework\DB\Select;
11+
use Magento\Framework\DB\Select\InQueryModifier;
12+
use Magento\Framework\Serialize\Serializer\Serialize;
13+
use Magento\TestFramework\Helper\Bootstrap;
14+
15+
class DataConverterTest extends \PHPUnit_Framework_TestCase
16+
{
17+
/**
18+
* @var \Magento\Framework\ObjectManagerInterface
19+
*/
20+
private $objectManager;
21+
22+
/**
23+
* @var InQueryModifier|\PHPUnit_Framework_MockObject_MockObject
24+
*/
25+
private $queryModifierMock;
26+
27+
/**
28+
* @var SerializedToJson
29+
*/
30+
private $dataConverter;
31+
32+
/**
33+
* @var \Magento\Framework\DB\Query\BatchIterator|\PHPUnit_Framework_MockObject_MockObject
34+
*/
35+
private $iteratorMock;
36+
37+
/**
38+
* @var \Magento\Framework\DB\Query\Generator|\PHPUnit_Framework_MockObject_MockObject
39+
*/
40+
private $queryGeneratorMock;
41+
42+
/**
43+
* @var Select|\PHPUnit_Framework_MockObject_MockObject
44+
*/
45+
private $selectByRangeMock;
46+
47+
/**
48+
* @var Mysql|\PHPUnit_Framework_MockObject_MockObject
49+
*/
50+
private $adapterMock;
51+
52+
/**
53+
* @var FieldDataConverter
54+
*/
55+
private $fieldDataConverter;
56+
57+
/**
58+
* Set up before test
59+
*/
60+
protected function setUp()
61+
{
62+
$this->objectManager = Bootstrap::getObjectManager();
63+
64+
/** @var InQueryModifier $queryModifier */
65+
$this->queryModifierMock = $this->getMockBuilder(Select\QueryModifierInterface::class)
66+
->disableOriginalConstructor()
67+
->setMethods(['modify'])
68+
->getMock();
69+
70+
$this->dataConverter = $this->objectManager->get(SerializedToJson::class);
71+
72+
$this->iteratorMock = $this->getMockBuilder(\Magento\Framework\DB\Query\BatchIterator::class)
73+
->disableOriginalConstructor()
74+
->setMethods(['current', 'valid', 'next'])
75+
->getMock();
76+
77+
$iterationComplete = false;
78+
79+
// mock valid() call so iterator passes only current() result in foreach invocation
80+
$this->iteratorMock->expects($this->any())->method('valid')->will(
81+
$this->returnCallback(
82+
function () use (&$iterationComplete) {
83+
if (!$iterationComplete) {
84+
$iterationComplete = true;
85+
return true;
86+
} else {
87+
return false;
88+
}
89+
}
90+
)
91+
);
92+
93+
$this->queryGeneratorMock = $this->getMockBuilder(\Magento\Framework\DB\Query\Generator::class)
94+
->disableOriginalConstructor()
95+
->setMethods(['generate'])
96+
->getMock();
97+
98+
$this->selectByRangeMock = $this->getMockBuilder(Select::class)
99+
->disableOriginalConstructor()
100+
->setMethods([])
101+
->getMock();
102+
103+
$this->queryGeneratorMock->expects($this->any())
104+
->method('generate')
105+
->will($this->returnValue($this->iteratorMock));
106+
107+
// mocking only current as next() is not supposed to be called
108+
$this->iteratorMock->expects($this->any())
109+
->method('current')
110+
->will($this->returnValue($this->selectByRangeMock));
111+
112+
$this->adapterMock = $this->getMockBuilder(Mysql::class)
113+
->disableOriginalConstructor()
114+
->setMethods(['fetchAll', 'quoteInto', 'update'])
115+
->getMock();
116+
117+
$this->adapterMock->expects($this->any())
118+
->method('quoteInto')
119+
->will($this->returnValue('field=value'));
120+
121+
$this->fieldDataConverter = $this->objectManager->create(
122+
FieldDataConverter::class,
123+
[
124+
'queryGenerator' => $this->queryGeneratorMock,
125+
'dataConverter' => $this->dataConverter
126+
]
127+
);
128+
}
129+
130+
/**
131+
* Test that exception with valid text is thrown when data is corrupted
132+
* @expectedException \Magento\Framework\DB\FieldDataConversionException
133+
* @expectedExceptionMessage Error converting field `value` in table `table` where `id`=2 using
134+
*/
135+
public function testDataConvertErrorReporting()
136+
{
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);
142+
$rows = [
143+
['id' => 1, 'value' => 'N;'],
144+
['id' => 2, 'value' => $brokenSerializedData],
145+
];
146+
147+
$this->adapterMock->expects($this->any())
148+
->method('fetchAll')
149+
->with($this->selectByRangeMock)
150+
->will($this->returnValue($rows));
151+
152+
$this->adapterMock->expects($this->once())
153+
->method('update')
154+
->with('table', ['value' => 'null'], ['id = ?' => 1]);
155+
156+
$this->fieldDataConverter->convert($this->adapterMock, 'table', 'id', 'value', $this->queryModifierMock);
157+
}
158+
159+
/**
160+
*/
161+
public function testAlreadyConvertedDataSkipped()
162+
{
163+
$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"}'],
171+
];
172+
173+
$this->adapterMock->expects($this->any())
174+
->method('fetchAll')
175+
->with($this->selectByRangeMock)
176+
->will($this->returnValue($rows));
177+
178+
$this->adapterMock->expects($this->once())
179+
->method('update')
180+
->with('table', ['value' => 'null'], ['id = ?' => 7]);
181+
182+
$this->fieldDataConverter->convert($this->adapterMock, 'table', 'id', 'value', $this->queryModifierMock);
183+
}
184+
}

dev/tests/static/testsuite/Magento/Test/Integrity/_files/blacklist/exception_hierarchy.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,5 @@
77
\Magento\Framework\DB\Adapter\DeadlockException
88
\Magento\Framework\DB\Adapter\LockWaitException
99
\Magento\Framework\DB\Adapter\DuplicateException
10+
\Magento\Framework\DB\DataConverter\DataConversionException
11+
\Magento\Framework\DB\FieldDataConversionException
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
/**
3+
* Copyright © 2013-2017 Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
namespace Magento\Framework\DB\DataConverter;
8+
9+
/**
10+
* Class DataConversionException.
11+
* @package Magento\Framework\DB\DataConverter
12+
*/
13+
class DataConversionException extends \Exception
14+
{
15+
16+
}

lib/internal/Magento/Framework/DB/DataConverter/DataConverterInterface.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ interface DataConverterInterface
1515
*
1616
* @param string $value
1717
* @return string
18+
*
19+
* @throws DataConversionException
1820
*/
1921
public function convert($value);
2022
}

0 commit comments

Comments
 (0)