Skip to content

Commit a30fcf9

Browse files
authored
ENGCOM-3130: add more CDATA-related tests for Magento\Framework\Config\Dom::merge and fix failing ones #18336
2 parents efd431e + 55c95f1 commit a30fcf9

23 files changed

+366
-1
lines changed

lib/internal/Magento/Framework/Config/Dom.php

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,20 @@ protected function _mergeNode(\DOMElement $node, $parentPath)
190190
/* override node value */
191191
if ($this->_isTextNode($node)) {
192192
/* skip the case when the matched node has children, otherwise they get overridden */
193-
if (!$matchedNode->hasChildNodes() || $this->_isTextNode($matchedNode)) {
193+
if (!$matchedNode->hasChildNodes()
194+
|| $this->_isTextNode($matchedNode)
195+
|| $this->isCdataNode($matchedNode)
196+
) {
194197
$matchedNode->nodeValue = $node->childNodes->item(0)->nodeValue;
195198
}
199+
} elseif ($this->isCdataNode($node) && $this->_isTextNode($matchedNode)) {
200+
/* Replace text node with CDATA section */
201+
if ($this->findCdataSection($node)) {
202+
$matchedNode->nodeValue = $this->findCdataSection($node)->nodeValue;
203+
}
204+
} elseif ($this->isCdataNode($node) && $this->isCdataNode($matchedNode)) {
205+
/* Replace CDATA with new one */
206+
$this->replaceCdataNode($matchedNode, $node);
196207
} else {
197208
/* recursive merge for all child nodes */
198209
foreach ($node->childNodes as $childNode) {
@@ -220,6 +231,56 @@ protected function _isTextNode($node)
220231
return $node->childNodes->length == 1 && $node->childNodes->item(0) instanceof \DOMText;
221232
}
222233

234+
/**
235+
* Check if the node content is CDATA (probably surrounded with text nodes) or just text node
236+
*
237+
* @param \DOMNode $node
238+
* @return bool
239+
*/
240+
private function isCdataNode($node)
241+
{
242+
// If every child node of current is NOT \DOMElement
243+
// It is arbitrary combination of text nodes and CDATA sections.
244+
foreach ($node->childNodes as $childNode) {
245+
if ($childNode instanceof \DOMElement) {
246+
return false;
247+
}
248+
}
249+
250+
return true;
251+
}
252+
253+
/**
254+
* Finds CDATA section from given node children
255+
*
256+
* @param \DOMNode $node
257+
* @return \DOMCdataSection|null
258+
*/
259+
private function findCdataSection($node)
260+
{
261+
foreach ($node->childNodes as $childNode) {
262+
if ($childNode instanceof \DOMCdataSection) {
263+
return $childNode;
264+
}
265+
}
266+
}
267+
268+
/**
269+
* Replaces CDATA section in $oldNode with $newNode's
270+
*
271+
* @param \DOMNode $oldNode
272+
* @param \DOMNode $newNode
273+
*/
274+
private function replaceCdataNode($oldNode, $newNode)
275+
{
276+
$oldCdata = $this->findCdataSection($oldNode);
277+
$newCdata = $this->findCdataSection($newNode);
278+
279+
if ($oldCdata && $newCdata) {
280+
$oldCdata->nodeValue = $newCdata->nodeValue;
281+
}
282+
}
283+
223284
/**
224285
* Merges attributes of the merge node to the base node
225286
*

lib/internal/Magento/Framework/Config/Test/Unit/DomTest.php

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
*/
66
namespace Magento\Framework\Config\Test\Unit;
77

8+
/**
9+
* Test for \Magento\Framework\Config\Dom class.
10+
*/
811
class DomTest extends \PHPUnit\Framework\TestCase
912
{
1013
/**
@@ -62,6 +65,37 @@ public function mergeDataProvider()
6265
['override_node.xml', 'override_node_new.xml', [], null, 'override_node_merged.xml'],
6366
['override_node_new.xml', 'override_node.xml', [], null, 'override_node_merged.xml'],
6467
['text_node.xml', 'text_node_new.xml', [], null, 'text_node_merged.xml'],
68+
'text node replaced with cdata' => [
69+
'text_node_cdata.xml',
70+
'text_node_cdata_new.xml',
71+
[],
72+
null,
73+
'text_node_cdata_merged.xml'
74+
],
75+
'cdata' => ['cdata.xml', 'cdata_new.xml', [], null, 'cdata_merged.xml'],
76+
'cdata with html' => ['cdata_html.xml', 'cdata_html_new.xml', [], null, 'cdata_html_merged.xml'],
77+
'cdata replaced with text node' => [
78+
'cdata_text.xml',
79+
'cdata_text_new.xml',
80+
[],
81+
null,
82+
'cdata_text_merged.xml'
83+
],
84+
'big cdata' => ['big_cdata.xml', 'big_cdata_new.xml', [], null, 'big_cdata_merged.xml'],
85+
'big cdata with attribute' => [
86+
'big_cdata_attribute.xml',
87+
'big_cdata_attribute_new.xml',
88+
[],
89+
null,
90+
'big_cdata_attribute_merged.xml'
91+
],
92+
'big cdata replaced with text' => [
93+
'big_cdata_text.xml',
94+
'big_cdata_text_new.xml',
95+
[],
96+
null,
97+
'big_cdata_text_merged.xml'
98+
],
6599
[
66100
'recursive.xml',
67101
'recursive_new.xml',
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?xml version="1.0"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
<root>
9+
<node>
10+
<subnode>
11+
<![CDATA[Some Phrase]]>
12+
</subnode>
13+
</node>
14+
</root>
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?xml version="1.0"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
<root>
9+
<node>
10+
<subnode attr="text">
11+
<![CDATA[Some Phrase]]>
12+
</subnode>
13+
</node>
14+
</root>
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?xml version="1.0"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
<root>
9+
<node>
10+
<subnode attr="text">
11+
<![CDATA[Some Other Phrase]]>
12+
</subnode>
13+
</node>
14+
</root>
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?xml version="1.0"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
<root>
9+
<node>
10+
<subnode>
11+
<![CDATA[Some Other Phrase]]>
12+
</subnode>
13+
</node>
14+
</root>
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?xml version="1.0"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
<root>
9+
<node>
10+
<subnode>
11+
<![CDATA[Some Other Phrase]]>
12+
</subnode>
13+
</node>
14+
</root>
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?xml version="1.0"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
<root>
9+
<node>
10+
<subnode>
11+
<![CDATA[Some Other Phrase]]>
12+
</subnode>
13+
</node>
14+
</root>
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?xml version="1.0"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
<root>
9+
<node>
10+
<subnode>
11+
<![CDATA[Some Phrase]]>
12+
</subnode>
13+
</node>
14+
</root>
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?xml version="1.0"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
<root>
9+
<node>
10+
<subnode>Some Other Phrase</subnode>
11+
</node>
12+
</root>

0 commit comments

Comments
 (0)