Skip to content

Commit 6fee6d9

Browse files
rganinrganin
committed
MAGETWO-64802: Implement corrupted data reporting for fieldDataConverter
- custom converters - fixed integration test and removed dependency
1 parent f6271c0 commit 6fee6d9

File tree

8 files changed

+231
-146
lines changed

8 files changed

+231
-146
lines changed

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,10 @@ class SerializedDataConverter extends SerializedToJson
2424
*/
2525
public function convert($value)
2626
{
27-
$valueUnserialized = parent::unserializeValue($value);
27+
if ($this->isValidJsonValue($value)) {
28+
return $value;
29+
}
30+
$valueUnserialized = $this->unserializeValue($value);
2831
if (isset($valueUnserialized['options'])) {
2932
foreach ($valueUnserialized['options'] as $key => $option) {
3033
if ($option['option_type'] === 'file') {
@@ -37,6 +40,6 @@ public function convert($value)
3740
$valueUnserialized['bundle_selection_attributes']
3841
);
3942
}
40-
return parent::encodeJson($valueUnserialized);
43+
return $this->encodeJson($valueUnserialized);
4144
}
4245
}

‎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
*

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

Lines changed: 135 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@
55
*/
66
namespace Magento\Framework\DB\DataConverter;
77

8-
use Magento\Framework\DB\Adapter\AdapterInterface;
8+
use Magento\Framework\DB\Adapter\Pdo\Mysql;
99
use Magento\Framework\DB\FieldDataConverter;
10+
use Magento\Framework\DB\Select;
1011
use Magento\Framework\DB\Select\InQueryModifier;
1112
use Magento\Framework\Serialize\Serializer\Serialize;
1213
use Magento\TestFramework\Helper\Bootstrap;
@@ -18,114 +19,166 @@ class DataConverterTest extends \PHPUnit_Framework_TestCase
1819
*/
1920
private $objectManager;
2021

21-
protected function setUp()
22-
{
23-
$this->objectManager = Bootstrap::getObjectManager();
24-
}
22+
/**
23+
* @var InQueryModifier|\PHPUnit_Framework_MockObject_MockObject
24+
*/
25+
private $queryModifierMock;
2526

2627
/**
27-
* @magentoAppArea adminhtml
28-
* @magentoDbIsolation enabled
29-
* @magentoDataFixture Magento/Framework/DB/DataConverter/_files/broken_admin_user.php
30-
* @expectedException \Magento\Setup\Exception
31-
* @expectedExceptionMessageRegExp
32-
* #Error converting field `extra` in table `admin_user` where `user_id`\=\d using.*#
28+
* @var SerializedToJson
3329
*/
34-
public function testDataConvertErrorReporting()
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()
3561
{
36-
/** @var \Magento\User\Model\User $user */
37-
$user = $this->objectManager->create(\Magento\User\Model\User::class);
38-
$user->loadByUsername('broken_admin');
39-
$userId = $user->getId();
62+
$this->objectManager = Bootstrap::getObjectManager();
4063

41-
/** @var Serialize $serializer */
42-
$serializer = $this->objectManager->create(Serialize::class);
43-
$serializedData = $serializer->serialize(['some' => 'data', 'other' => 'other data']);
44-
$serializedDataLength = strlen($serializedData);
45-
$brokenSerializedData = substr($serializedData, 0, $serializedDataLength - 6);
46-
/** @var AdapterInterface $adapter */
47-
$adapter = $user->getResource()->getConnection();
48-
$adapter->update(
49-
$user->getResource()->getTable('admin_user'),
50-
['extra' => $brokenSerializedData],
51-
"user_id={$userId}"
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+
)
5291
);
5392

54-
/** @var InQueryModifier $queryModifier */
55-
$queryModifier = $this->objectManager->create(InQueryModifier::class, ['user_id' => $userId]);
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();
56102

57-
/** @var SerializedToJson $dataConverter */
58-
$dataConverter = $this->objectManager->get(SerializedToJson::class);
103+
$this->queryGeneratorMock->expects($this->any())
104+
->method('generate')
105+
->will($this->returnValue($this->iteratorMock));
59106

60-
/** @var FieldDataConverter $fieldDataConverter */
61-
$fieldDataConverter = $this->objectManager->create(
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(
62122
FieldDataConverter::class,
63123
[
64-
'dataConverter' => $dataConverter
124+
'queryGenerator' => $this->queryGeneratorMock,
125+
'dataConverter' => $this->dataConverter
65126
]
66127
);
67-
$fieldDataConverter->convert(
68-
$adapter,
69-
'admin_user',
70-
'user_id',
71-
'extra',
72-
$queryModifier
73-
);
74128
}
75129

76130
/**
77-
* @magentoAppArea adminhtml
78-
* @magentoDbIsolation enabled
79-
* @magentoDataFixture Magento/Framework/DB/DataConverter/_files/broken_admin_user.php
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
80134
*/
81-
public function testAlreadyConvertedDataSkipped()
135+
public function testDataConvertErrorReporting()
82136
{
83-
/** @var \Magento\User\Model\User $user */
84-
$user = $this->objectManager->create(\Magento\User\Model\User::class);
85-
$user->loadByUsername('broken_admin');
86-
$userId = $user->getId();
87-
88137
/** @var Serialize $serializer */
89138
$serializer = $this->objectManager->create(Serialize::class);
90139
$serializedData = $serializer->serialize(['some' => 'data', 'other' => 'other data']);
91140
$serializedDataLength = strlen($serializedData);
92141
$brokenSerializedData = substr($serializedData, 0, $serializedDataLength - 6);
93-
/** @var AdapterInterface $adapter */
94-
$adapter = $user->getResource()->getConnection();
95-
$adapter->update(
96-
$user->getResource()->getTable('admin_user'),
97-
['extra' => $brokenSerializedData],
98-
"user_id={$userId}"
99-
);
142+
$rows = [
143+
['id' => 1, 'value' => 'N;'],
144+
['id' => 2, 'value' => $brokenSerializedData],
145+
];
100146

101-
$adapter->update(
102-
$user->getResource()->getTable('admin_user'),
103-
['extra' => '[]'],
104-
"user_id={$userId}"
105-
);
147+
$this->adapterMock->expects($this->any())
148+
->method('fetchAll')
149+
->with($this->selectByRangeMock)
150+
->will($this->returnValue($rows));
106151

107-
/** @var InQueryModifier $queryModifier */
108-
$queryModifier = $this->objectManager->create(InQueryModifier::class, ['user_id' => $userId]);
152+
$this->adapterMock->expects($this->once())
153+
->method('update')
154+
->with('table', ['value' => 'null'], ['id = ?' => 1]);
109155

110-
/** @var SerializedToJson $dataConverter */
111-
$dataConverter = $this->objectManager->get(SerializedToJson::class);
156+
$this->fieldDataConverter->convert($this->adapterMock, 'table', 'id', 'value', $this->queryModifierMock);
157+
}
112158

113-
/** @var FieldDataConverter $fieldDataConverter */
114-
$fieldDataConverter = $this->objectManager->create(
115-
FieldDataConverter::class,
116-
[
117-
'dataConverter' => $dataConverter
118-
]
119-
);
120-
$fieldDataConverter->convert(
121-
$adapter,
122-
'admin_user',
123-
'user_id',
124-
'extra',
125-
$queryModifier
126-
);
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]);
127181

128-
$user->load($userId);
129-
$this->assertEquals([], $user->getExtra());
182+
$this->fieldDataConverter->convert($this->adapterMock, 'table', 'id', 'value', $this->queryModifierMock);
130183
}
131184
}

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

Lines changed: 0 additions & 26 deletions
This file was deleted.

‎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)