Skip to content

Commit 0c5e6ee

Browse files
author
Joan He
committed
MAGETWO-62253: Refactor code to remove unserialize
- fixed unit test failures - add unit test - fixed static test failure
1 parent d79dfea commit 0c5e6ee

File tree

3 files changed

+246
-36
lines changed

3 files changed

+246
-36
lines changed

‎app/code/Magento/Backend/Model/Menu/Item.php

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ class Item
131131
* Serialized submenu string
132132
*
133133
* @var string
134+
* @deprecated
134135
*/
135136
protected $_serializedSubmenu;
136137

@@ -169,14 +170,14 @@ public function __construct(
169170
$this->_validator = $validator;
170171
if (!empty($data)) {
171172
$this->_validator->validate($data);
172-
$this->populateFromArray($data);
173173
}
174174
$this->_moduleManager = $moduleManager;
175175
$this->_acl = $authorization;
176176
$this->_scopeConfig = $scopeConfig;
177177
$this->_menuFactory = $menuFactory;
178178
$this->_urlModel = $urlModel;
179179
$this->_moduleList = $moduleList;
180+
$this->populateFromArray($data);
180181
}
181182

182183
/**
@@ -447,7 +448,7 @@ public function toArray()
447448
{
448449
return [
449450
'parent_id' => $this->_parentId,
450-
'module' => $this->_moduleName,
451+
'module_name' => $this->_moduleName,
451452
'sort_index' => $this->_sortIndex,
452453
'depends_on_config' => $this->_dependsOnConfig,
453454
'id' => $this->_id,
@@ -469,20 +470,17 @@ public function toArray()
469470
*/
470471
public function populateFromArray(array $data)
471472
{
472-
$this->_moduleName = isset($data['module']) ? $data['module'] : 'Magento_Backend';
473-
474-
$this->_id = $data['id'];
475-
$this->_title = $data['title'];
476-
$this->_action = $this->_getArgument($data, 'action');
477-
$this->_resource = $this->_getArgument($data, 'resource');
478-
$this->_dependsOnModule = $this->_getArgument($data, 'dependsOnModule');
479-
$this->_dependsOnConfig = $this->_getArgument($data, 'dependsOnConfig');
480-
$this->_tooltip = $this->_getArgument($data, 'toolTip', '');
481473
$this->_parentId = $this->_getArgument($data, 'parent_id');
474+
$this->_moduleName = $this->_getArgument($data, 'module_name', 'Magento_Backend');
482475
$this->_sortIndex = $this->_getArgument($data, 'sort_index');
483476
$this->_dependsOnConfig = $this->_getArgument($data, 'depends_on_config');
484-
$this->_path = $this->_getArgument($data, 'path');
477+
$this->_id = $this->_getArgument($data, 'id');
478+
$this->_resource = $this->_getArgument($data, 'resource');
479+
$this->_path = $this->_getArgument($data, 'path', '');
480+
$this->_action = $this->_getArgument($data, 'action');
485481
$this->_dependsOnModule = $this->_getArgument($data, 'depends_on_module');
482+
$this->_tooltip = $this->_getArgument($data, 'tooltip', '');
483+
$this->_title = $this->_getArgument($data, 'title');
486484
if (isset($data['sub_menu'])) {
487485
$menu = $this->_menuFactory->create();
488486
$menu->populateFromArray($data['sub_menu']);

‎app/code/Magento/Backend/Test/Unit/Model/Menu/ConfigTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ protected function setUp()
7171

7272
$this->menuBuilderMock = $this->getMock(\Magento\Backend\Model\Menu\Builder::class, [], [], '', false);
7373

74-
$menuFactoryMock->expects($this->any())->method('create')->will($this->returnValue($this->menuMock));;
74+
$menuFactoryMock->expects($this->any())->method('create')->will($this->returnValue($this->menuMock));
7575

7676
$this->configReaderMock->expects($this->any())->method('read')->will($this->returnValue([]));
7777

‎app/code/Magento/Backend/Test/Unit/Model/Menu/ItemTest.php

Lines changed: 235 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -40,16 +40,14 @@ class ItemTest extends \PHPUnit_Framework_TestCase
4040
*/
4141
protected $_moduleManager;
4242

43-
/**
44-
* @var \PHPUnit_Framework_MockObject_MockObject
45-
*/
46-
protected $_validatorMock;
47-
4843
/**
4944
* @var \PHPUnit_Framework_MockObject_MockObject
5045
*/
5146
protected $_moduleListMock;
5247

48+
/** @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager */
49+
private $objectManager;
50+
5351
/**
5452
* @var array
5553
*/
@@ -58,8 +56,8 @@ class ItemTest extends \PHPUnit_Framework_TestCase
5856
'title' => 'Item Title',
5957
'action' => '/system/config',
6058
'resource' => 'Magento_Config::config',
61-
'dependsOnModule' => 'Magento_Backend',
62-
'dependsOnConfig' => 'system/config/isEnabled',
59+
'depends_on_module' => 'Magento_Backend',
60+
'depends_on_config' => 'system/config/isEnabled',
6361
'tooltip' => 'Item tooltip',
6462
];
6563

@@ -76,15 +74,15 @@ protected function setUp()
7674
);
7775
$this->_urlModelMock = $this->getMock(\Magento\Backend\Model\Url::class, [], [], '', false);
7876
$this->_moduleManager = $this->getMock(\Magento\Framework\Module\Manager::class, [], [], '', false);
79-
$this->_validatorMock = $this->getMock(\Magento\Backend\Model\Menu\Item\Validator::class);
80-
$this->_validatorMock->expects($this->any())->method('validate');
77+
$validatorMock = $this->getMock(\Magento\Backend\Model\Menu\Item\Validator::class);
78+
$validatorMock->expects($this->any())->method('validate');
8179
$this->_moduleListMock = $this->getMock(\Magento\Framework\Module\ModuleListInterface::class);
8280

83-
$helper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
84-
$this->_model = $helper->getObject(
81+
$this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
82+
$this->_model = $this->objectManager->getObject(
8583
\Magento\Backend\Model\Menu\Item::class,
8684
[
87-
'validator' => $this->_validatorMock,
85+
'validator' => $validatorMock,
8886
'authorization' => $this->_aclMock,
8987
'scopeConfig' => $this->_scopeConfigMock,
9088
'menuFactory' => $this->_menuFactoryMock,
@@ -99,8 +97,7 @@ protected function setUp()
9997
public function testGetUrlWithEmptyActionReturnsHashSign()
10098
{
10199
$this->_params['action'] = '';
102-
$helper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
103-
$item = $helper->getObject(
100+
$item = $this->objectManager->getObject(
104101
\Magento\Backend\Model\Menu\Item::class,
105102
['menuFactory' => $this->_menuFactoryMock, 'data' => $this->_params]
106103
);
@@ -129,8 +126,7 @@ public function testHasClickCallbackReturnsFalseIfItemHasAction()
129126
public function testHasClickCallbackReturnsTrueIfItemHasNoAction()
130127
{
131128
$this->_params['action'] = '';
132-
$helper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
133-
$item = $helper->getObject(
129+
$item = $this->objectManager->getObject(
134130
\Magento\Backend\Model\Menu\Item::class,
135131
['menuFactory' => $this->_menuFactoryMock, 'data' => $this->_params]
136132
);
@@ -140,8 +136,7 @@ public function testHasClickCallbackReturnsTrueIfItemHasNoAction()
140136
public function testGetClickCallbackReturnsStoppingJsIfItemDoesntHaveAction()
141137
{
142138
$this->_params['action'] = '';
143-
$helper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
144-
$item = $helper->getObject(
139+
$item = $this->objectManager->getObject(
145140
\Magento\Backend\Model\Menu\Item::class,
146141
['menuFactory' => $this->_menuFactoryMock, 'data' => $this->_params]
147142
);
@@ -218,15 +213,232 @@ public function testIsAllowedReturnsFalseIfResourceIsNotAvailable()
218213

219214
public function testGetChildrenCreatesSubmenuOnFirstCall()
220215
{
221-
$menuMock = $this->getMock(
222-
\Magento\Backend\Model\Menu::class,
223-
[],
224-
[$this->getMock(\Psr\Log\LoggerInterface::class)]
225-
);
216+
$menuMock = $this->getMock(\Magento\Backend\Model\Menu::class, [], [], '', false);
226217

227218
$this->_menuFactoryMock->expects($this->once())->method('create')->will($this->returnValue($menuMock));
228219

229220
$this->_model->getChildren();
230221
$this->_model->getChildren();
231222
}
223+
224+
/**
225+
* @param array $data
226+
* @param array $expected
227+
* @dataProvider toArrayDataProvider
228+
*/
229+
public function testToArray($data, $expected)
230+
{
231+
$menuMock = $this->getMock(\Magento\Backend\Model\Menu::class, [], [], '', false);
232+
$this->_menuFactoryMock->method('create')->will($this->returnValue($menuMock));
233+
$menuMock->method('toArray')
234+
->willReturn(isset($data['sub_menu']) ? $data['sub_menu'] : null);
235+
236+
$model = $this->objectManager->getObject(
237+
\Magento\Backend\Model\Menu\Item::class,
238+
[
239+
'authorization' => $this->_aclMock,
240+
'scopeConfig' => $this->_scopeConfigMock,
241+
'menuFactory' => $this->_menuFactoryMock,
242+
'urlModel' => $this->_urlModelMock,
243+
'moduleList' => $this->_moduleListMock,
244+
'moduleManager' => $this->_moduleManager,
245+
'data' => $data
246+
]
247+
);
248+
$this->assertEquals($expected, $model->toArray());
249+
}
250+
251+
public function toArrayDataProvider()
252+
{
253+
return [
254+
[
255+
$this->_params,
256+
[
257+
'parent_id' => null,
258+
'module_name' => 'Magento_Backend',
259+
'sort_index' => null,
260+
'depends_on_config' => 'system/config/isEnabled',
261+
'id' => 'item',
262+
'resource' => 'Magento_Config::config',
263+
'path' => '',
264+
'action' => '/system/config',
265+
'depends_on_module' => 'Magento_Backend',
266+
'tooltip' => 'Item tooltip',
267+
'title' => 'Item Title',
268+
'sub_menu' => null
269+
]
270+
],
271+
[
272+
[
273+
'parent_id' => '1',
274+
'module_name' => 'Magento_Module1',
275+
'sort_index' => '50',
276+
'depends_on_config' => null,
277+
'id' => '5',
278+
'resource' => null,
279+
'path' => null,
280+
'action' => null,
281+
'depends_on_module' => null,
282+
'tooltip' => null,
283+
'title' => null,
284+
'sub_menu' => $this->_params,
285+
],
286+
[
287+
'parent_id' => '1',
288+
'module_name' => 'Magento_Module1',
289+
'sort_index' => '50',
290+
'depends_on_config' => null,
291+
'id' => '5',
292+
'resource' => null,
293+
'path' => null,
294+
'action' => null,
295+
'depends_on_module' => null,
296+
'tooltip' => '',
297+
'title' => null,
298+
'sub_menu' => $this->_params
299+
]
300+
],
301+
[
302+
[
303+
'parent_id' => '1',
304+
'module_name' => 'Magento_Module1',
305+
'sort_index' => '50',
306+
'sub_menu' => $this->_params,
307+
],
308+
[
309+
'parent_id' => '1',
310+
'module_name' => 'Magento_Module1',
311+
'sort_index' => '50',
312+
'depends_on_config' => null,
313+
'id' => null,
314+
'resource' => null,
315+
'path' => '',
316+
'action' => null,
317+
'depends_on_module' => null,
318+
'tooltip' => '',
319+
'title' => null,
320+
'sub_menu' => $this->_params
321+
]
322+
]
323+
];
324+
}
325+
326+
/**
327+
* @param array $constructorData
328+
* @param array $populateFromData
329+
* @dataProvider populateFromArrayDataProvider
330+
*/
331+
public function testPopulateFromArray($constructorData, $populateFromData, $expected)
332+
{
333+
$menuMock = $this->getMock(\Magento\Backend\Model\Menu::class, [], [], '', false);
334+
$this->_menuFactoryMock->method('create')->will($this->returnValue($menuMock));
335+
$menuMock->method('toArray')
336+
->willReturn(isset($constructorData['sub_menu']) ? $constructorData['sub_menu'] : null);
337+
338+
$model = $this->objectManager->getObject(
339+
\Magento\Backend\Model\Menu\Item::class,
340+
[
341+
'authorization' => $this->_aclMock,
342+
'scopeConfig' => $this->_scopeConfigMock,
343+
'menuFactory' => $this->_menuFactoryMock,
344+
'urlModel' => $this->_urlModelMock,
345+
'moduleList' => $this->_moduleListMock,
346+
'moduleManager' => $this->_moduleManager,
347+
'data' => $constructorData
348+
]
349+
);
350+
$model->populateFromArray($populateFromData);
351+
$this->assertEquals($expected, $model->toArray());
352+
}
353+
354+
public function populateFromArrayDataProvider()
355+
{
356+
return [
357+
[
358+
[],
359+
$this->_params,
360+
[
361+
'parent_id' => null,
362+
'module_name' => 'Magento_Backend',
363+
'sort_index' => null,
364+
'depends_on_config' => 'system/config/isEnabled',
365+
'id' => 'item',
366+
'resource' => 'Magento_Config::config',
367+
'path' => '',
368+
'action' => '/system/config',
369+
'depends_on_module' => 'Magento_Backend',
370+
'tooltip' => 'Item tooltip',
371+
'title' => 'Item Title',
372+
'sub_menu' => null
373+
],
374+
],
375+
[
376+
$this->_params,
377+
[
378+
'parent_id' => '1',
379+
'module_name' => 'Magento_Module1',
380+
'sort_index' => '50',
381+
'depends_on_config' => null,
382+
'id' => '5',
383+
'resource' => null,
384+
'path' => null,
385+
'action' => null,
386+
'depends_on_module' => null,
387+
'tooltip' => null,
388+
'title' => null,
389+
'sub_menu' => $this->_params,
390+
],
391+
[
392+
'parent_id' => '1',
393+
'module_name' => 'Magento_Module1',
394+
'sort_index' => '50',
395+
'depends_on_config' => null,
396+
'id' => '5',
397+
'resource' => null,
398+
'path' => '',
399+
'action' => null,
400+
'depends_on_module' => null,
401+
'tooltip' => '',
402+
'title' => null,
403+
'sub_menu' => null
404+
],
405+
],
406+
[
407+
[
408+
'parent_id' => '1',
409+
'module_name' => 'Magento_Module1',
410+
'sort_index' => '50',
411+
'depends_on_config' => null,
412+
'id' => '5',
413+
'resource' => null,
414+
'path' => null,
415+
'action' => null,
416+
'depends_on_module' => null,
417+
'tooltip' => null,
418+
'title' => null,
419+
'sub_menu' => $this->_params,
420+
],
421+
[
422+
'parent_id' => '1',
423+
'module_name' => 'Magento_Module1',
424+
'sort_index' => '50',
425+
'sub_menu' => $this->_params,
426+
],
427+
[
428+
'parent_id' => '1',
429+
'module_name' => 'Magento_Module1',
430+
'sort_index' => '50',
431+
'depends_on_config' => null,
432+
'id' => null,
433+
'resource' => null,
434+
'path' => '',
435+
'action' => null,
436+
'depends_on_module' => null,
437+
'tooltip' => '',
438+
'title' => null,
439+
'sub_menu' => $this->_params
440+
],
441+
]
442+
];
443+
}
232444
}

0 commit comments

Comments
 (0)