Skip to content

Commit cdf6968

Browse files
author
Yurii Torbyk
committed
MAGETWO-33602: Child containers still present after removing parent container in page layout
1 parent 5d2ccb5 commit cdf6968

File tree

6 files changed

+148
-61
lines changed

6 files changed

+148
-61
lines changed

‎lib/internal/Magento/Framework/Data/Test/Unit/StructureTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ public function testUnsetElement()
206206
$this->assertSame([5], $this->_structure->getElement('five'));
207207

208208
// recursively
209+
$this->assertTrue($this->_structure->unsetElement('three'));
209210
$this->assertTrue($this->_structure->unsetElement('four'));
210211
$this->assertSame(['one' => [], 'five' => [5]], $this->_structure->exportElements());
211212
}

‎lib/internal/Magento/Framework/View/Layout/GeneratorPool.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ protected function moveElementInStructure(
211211
} catch (\OutOfBoundsException $e) {
212212
$this->logger->critical('Broken reference: '. $e->getMessage());
213213
}
214-
$scheduledStructure->unsetElementFromListToRemove($element);
214+
$scheduledStructure->unsetElementFromBrokenParentList($element);
215215
return $this;
216216
}
217217
}

‎lib/internal/Magento/Framework/View/Layout/ScheduledStructure.php

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,13 @@ class ScheduledStructure
5959
*/
6060
protected $_scheduledPaths;
6161

62+
/**
63+
* Elements with reference to non-existing parent element
64+
*
65+
* @var array
66+
*/
67+
protected $_brokenParent = [];
68+
6269
/**
6370
* @param array $data
6471
*
@@ -92,7 +99,10 @@ public function getListToMove()
9299
*/
93100
public function getListToRemove()
94101
{
95-
return array_keys(array_intersect_key($this->_scheduledElements, $this->_scheduledRemoves));
102+
return array_keys(array_intersect_key(
103+
$this->_scheduledElements,
104+
array_merge($this->_scheduledRemoves, $this->_brokenParent)
105+
));
96106
}
97107

98108
/**
@@ -399,6 +409,27 @@ public function unsetPathElement($elementName)
399409
unset($this->_scheduledPaths[$elementName]);
400410
}
401411

412+
/**
413+
* Remove element from broken parent list
414+
*
415+
* @param string $elementName
416+
* @return void
417+
*/
418+
public function unsetElementFromBrokenParentList($elementName)
419+
{
420+
unset($this->_brokenParent[$elementName]);
421+
}
422+
423+
/**
424+
* Set element to broken parent list
425+
*
426+
* @param string $elementName
427+
*/
428+
public function setElementToBrokenParentList($elementName)
429+
{
430+
$this->_brokenParent[$elementName] = 1;
431+
}
432+
402433
/**
403434
* Flush scheduled paths list
404435
*

‎lib/internal/Magento/Framework/View/Layout/ScheduledStructure/Helper.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ public function scheduleElement(
189189
$this->logger->critical($e);
190190
}
191191
} else {
192-
$scheduledStructure->setElementToRemoveList($key);
192+
$scheduledStructure->setElementToBrokenParentList($key);
193193
$this->logger->critical(
194194
"Broken reference: the '{$name}' element cannot be added as child to '{$parentName}', " .
195195
'because the latter doesn\'t exist'

‎lib/internal/Magento/Framework/View/Test/Unit/Layout/ScheduledStructure/HelperTest.php

Lines changed: 94 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,42 @@
1414
*/
1515
class HelperTest extends \PHPUnit_Framework_TestCase
1616
{
17+
/**
18+
* @var \Magento\Framework\View\Layout\ScheduledStructure|\PHPUnit_Framework_MockObject_MockObject
19+
*/
20+
protected $scheduledStructure;
21+
22+
/**
23+
* @var \Magento\Framework\View\Layout\Data\Structure|\PHPUnit_Framework_MockObject_MockObject
24+
*/
25+
protected $dataStructure;
26+
27+
/**
28+
* @var Helper
29+
*/
30+
protected $helper;
31+
32+
public function setUp()
33+
{
34+
$this->scheduledStructure = $this->getMockBuilder('Magento\Framework\View\Layout\ScheduledStructure')
35+
->disableOriginalConstructor()
36+
->getMock();
37+
38+
$this->dataStructure = $this->getMockBuilder('Magento\Framework\View\Layout\Data\Structure')
39+
->disableOriginalConstructor()
40+
->getMock();
41+
42+
$helperObjectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
43+
$this->helper = $helperObjectManager->getObject('Magento\Framework\View\Layout\ScheduledStructure\Helper');
44+
}
45+
1746
/**
1847
* @param string $currentNodeName
1948
* @param string $actualNodeName
20-
* @param \PHPUnit_Framework_MockObject_Matcher_InvokedCount $unsetPathElementCount
21-
* @param \PHPUnit_Framework_MockObject_Matcher_InvokedCount $unsetStructureElementCount
22-
* @dataProvider providerScheduleStructure
49+
* @param int $unsetPathElementCount
50+
* @param int $unsetStructureElementCount
51+
*
52+
* @dataProvider scheduleStructureDataProvider
2353
*/
2454
public function testScheduleStructure(
2555
$currentNodeName,
@@ -34,73 +64,67 @@ public function testScheduleStructure(
3464
$testPath = 'test_path';
3565
$potentialChild = 'potential_child';
3666

37-
/** @var Layout\ScheduledStructure|\PHPUnit_Framework_MockObject_MockObject $scheduledStructure */
38-
$scheduledStructure = $this->getMock('Magento\Framework\View\Layout\ScheduledStructure', [], [], '', false);
39-
$scheduledStructure->expects($this->once())->method('hasPath')
67+
$this->scheduledStructure->expects($this->once())->method('hasPath')
4068
->with($parentNodeName)
4169
->will($this->returnValue(true));
42-
$scheduledStructure->expects($this->any())->method('hasStructureElement')
70+
$this->scheduledStructure->expects($this->any())->method('hasStructureElement')
4371
->with($actualNodeName)
4472
->will($this->returnValue(true));
45-
$scheduledStructure->expects($this->once())->method('setPathElement')
73+
$this->scheduledStructure->expects($this->once())->method('setPathElement')
4674
->with($actualNodeName, $testPath . '/' . $actualNodeName)
4775
->will($this->returnValue(true));
48-
$scheduledStructure->expects($this->once())->method('setStructureElement')
76+
$this->scheduledStructure->expects($this->once())->method('setStructureElement')
4977
->with($actualNodeName, [$block, $currentNodeAs, $parentNodeName, $after, true]);
50-
$scheduledStructure->expects($this->once())->method('getPath')
78+
$this->scheduledStructure->expects($this->once())->method('getPath')
5179
->with($parentNodeName)
5280
->will($this->returnValue('test_path'));
53-
$scheduledStructure->expects($this->once())->method('getPaths')
81+
$this->scheduledStructure->expects($this->once())->method('getPaths')
5482
->will($this->returnValue([$potentialChild => $testPath . '/' . $currentNodeName . '/']));
55-
$scheduledStructure->expects($unsetPathElementCount)->method('unsetPathElement')
83+
$this->scheduledStructure->expects($this->exactly($unsetPathElementCount))->method('unsetPathElement')
5684
->with($potentialChild);
57-
$scheduledStructure->expects($unsetStructureElementCount)->method('unsetStructureElement')
85+
$this->scheduledStructure->expects($this->exactly($unsetStructureElementCount))->method('unsetStructureElement')
5886
->with($potentialChild);
5987

6088
$currentNode = new \Magento\Framework\View\Layout\Element(
6189
'<' . $block . ' name="' . $currentNodeName . '" as="' . $currentNodeAs . '" after="' . $after . '"/>'
6290
);
6391
$parentNode = new \Magento\Framework\View\Layout\Element('<' . $block . ' name="' . $parentNodeName . '"/>');
6492

65-
/** @var Layout\ScheduledStructure\Helper $helper */
66-
$helper = (new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this))
67-
->getObject('Magento\Framework\View\Layout\ScheduledStructure\Helper');
68-
$result = $helper->scheduleStructure($scheduledStructure, $currentNode, $parentNode);
93+
$result = $this->helper->scheduleStructure($this->scheduledStructure, $currentNode, $parentNode);
6994
$this->assertEquals($actualNodeName, $result);
7095
}
7196

7297
/**
7398
* @return array
7499
*/
75-
public function providerScheduleStructure()
100+
public function scheduleStructureDataProvider()
76101
{
77102
return [
78-
['current_node', 'current_node', $this->once(), $this->once()],
79-
['', 'parent_node_schedule_block0', $this->never(), $this->never()]
103+
['current_node', 'current_node', 1, 1],
104+
['', 'parent_node_schedule_block0', 0, 0]
80105
];
81106
}
82107

83108
public function testScheduleNonExistentElement()
84109
{
85110
$key = 'key';
86111

87-
/** @var Layout\ScheduledStructure|\PHPUnit_Framework_MockObject_MockObject $scheduledStructure */
88-
$scheduledStructure = $this->getMock('Magento\Framework\View\Layout\ScheduledStructure', [], [], '', false);
89-
$scheduledStructure->expects($this->once())->method('getStructureElement')->with($key)
112+
$this->scheduledStructure->expects($this->once())->method('getStructureElement')->with($key)
90113
->willReturn([]);
91-
$scheduledStructure->expects($this->once())->method('unsetPathElement')->with($key);
92-
$scheduledStructure->expects($this->once())->method('unsetStructureElement')->with($key);
114+
$this->scheduledStructure->expects($this->once())->method('unsetPathElement')->with($key);
115+
$this->scheduledStructure->expects($this->once())->method('unsetStructureElement')->with($key);
93116

94-
/** @var Layout\Data\Structure|\PHPUnit_Framework_MockObject_MockObject $scheduledStructure */
95-
$dataStructure = $this->getMock('Magento\Framework\View\Layout\Data\Structure', [], [], '', false);
96-
97-
/** @var Layout\ScheduledStructure\Helper $helper */
98-
$helper = (new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this))
99-
->getObject('Magento\Framework\View\Layout\ScheduledStructure\Helper');
100-
$helper->scheduleElement($scheduledStructure, $dataStructure, $key);
117+
$this->helper->scheduleElement($this->scheduledStructure, $this->dataStructure, $key);
101118
}
102119

103-
public function testScheduleElement()
120+
/**
121+
* @param bool $hasParent
122+
* @param int $setAsChild
123+
* @param int $toRemoveList
124+
*
125+
* @dataProvider scheduleElementDataProvider
126+
*/
127+
public function testScheduleElement($hasParent, $setAsChild, $toRemoveList)
104128
{
105129
$key = 'key';
106130
$parentName = 'parent';
@@ -109,10 +133,9 @@ public function testScheduleElement()
109133
$block = 'block';
110134
$data = ['data'];
111135

112-
/** @var Layout\ScheduledStructure|\PHPUnit_Framework_MockObject_MockObject $scheduledStructure */
113-
$scheduledStructure = $this->getMock('Magento\Framework\View\Layout\ScheduledStructure', [], [], '', false);
114-
$scheduledStructure->expects($this->any())->method('getStructureElement')->will(
115-
$this->returnValueMap(
136+
$this->scheduledStructure->expects($this->any())
137+
->method('getStructureElement')
138+
->willReturnMap(
116139
[
117140
[
118141
$key,
@@ -127,27 +150,40 @@ public function testScheduleElement()
127150
],
128151
[$parentName, null, []],
129152
]
130-
)
131-
);
132-
$scheduledStructure->expects($this->any())->method('getStructureElementData')->will(
133-
$this->returnValueMap([
134-
[$key, null, $data],
135-
[$parentName, null, $data],
136-
])
137-
);
138-
$scheduledStructure->expects($this->any())->method('hasStructureElement')->will($this->returnValue(true));
139-
$scheduledStructure->expects($this->once())->method('setElement')->with($key, [$block, $data]);
140-
141-
/** @var Layout\Data\Structure|\PHPUnit_Framework_MockObject_MockObject $scheduledStructure */
142-
$dataStructure = $this->getMock('Magento\Framework\View\Layout\Data\Structure', [], [], '', false);
143-
$dataStructure->expects($this->once())->method('createElement')->with($key, ['type' => $block]);
144-
$dataStructure->expects($this->once())->method('hasElement')->with($parentName)->will($this->returnValue(true));
145-
$dataStructure->expects($this->once())->method('setAsChild')->with($key, $parentName, $alias)
146-
->will($this->returnValue(true));
153+
);
154+
$this->scheduledStructure->expects($this->any())
155+
->method('getStructureElementData')
156+
->willReturnMap(
157+
[
158+
[$key, null, $data],
159+
[$parentName, null, $data],
160+
]
161+
);
162+
$this->scheduledStructure->expects($this->any())->method('hasStructureElement')->willReturn(true);
163+
$this->scheduledStructure->expects($this->once())->method('setElement')->with($key, [$block, $data]);
164+
165+
$this->dataStructure->expects($this->once())->method('createElement')->with($key, ['type' => $block]);
166+
$this->dataStructure->expects($this->once())->method('hasElement')->with($parentName)->willReturn($hasParent);
167+
$this->dataStructure->expects($this->exactly($setAsChild))
168+
->method('setAsChild')
169+
->with($key, $parentName, $alias)
170+
->willReturn(true);
171+
172+
$this->scheduledStructure->expects($this->exactly($toRemoveList))
173+
->method('setElementToBrokenParentList')
174+
->with($key);
147175

148-
/** @var Layout\ScheduledStructure\Helper $helper */
149-
$helper = (new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this))
150-
->getObject('Magento\Framework\View\Layout\ScheduledStructure\Helper');
151-
$helper->scheduleElement($scheduledStructure, $dataStructure, $key);
176+
$this->helper->scheduleElement($this->scheduledStructure, $this->dataStructure, $key);
177+
}
178+
179+
/**
180+
* @return array
181+
*/
182+
public function scheduleElementDataProvider()
183+
{
184+
return [
185+
['hasParent' => true, 'setAsChild' => 1, 'toRemoveList' => 0],
186+
['hasParent' => false, 'setAsChild' => 0, 'toRemoveList' => 1],
187+
];
152188
}
153189
}

‎lib/internal/Magento/Framework/View/Test/Unit/Layout/ScheduledStructureTest.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ protected function setUp()
3636
'element3' => ['data', 'of', 'element', '3'],
3737
'element4' => ['data', 'of', 'element', '4'],
3838
'element5' => ['data', 'of', 'element', '5'],
39+
'element9' => ['data', 'of', 'element', '9'],
3940
],
4041
'scheduledMoves' => [
4142
'element1' => ['data', 'of', 'element', 'to', 'move', '1'],
@@ -383,4 +384,22 @@ public function testFlushScheduledStructure()
383384
$this->assertEmpty($this->_model->getElements());
384385
$this->assertEmpty($this->_model->getStructure());
385386
}
387+
388+
/**
389+
* covers \Magento\Framework\View\Layout\ScheduledStructure::setElementToBrokenParentList
390+
* covers \Magento\Framework\View\Layout\ScheduledStructure::unsetElementFromBrokenParentList
391+
*/
392+
public function testSetElementToBrokenParentList()
393+
{
394+
$element = 'element9';
395+
$expectedToRemove = ['element2', 'element3'];
396+
$expectedToRemoveWithBroken = ['element2', 'element3', $element];
397+
$this->assertEquals($expectedToRemove, $this->_model->getListToRemove());
398+
399+
$this->_model->setElementToBrokenParentList($element);
400+
$this->assertEquals($expectedToRemoveWithBroken, $this->_model->getListToRemove());
401+
402+
$this->_model->unsetElementFromBrokenParentList($element);
403+
$this->assertEquals($expectedToRemove, $this->_model->getListToRemove());
404+
}
386405
}

0 commit comments

Comments
 (0)