Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add more CDATA-related tests for Magento\Framework\Config\Dom::merge and fix failing ones #18336

Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 62 additions & 1 deletion lib/internal/Magento/Framework/Config/Dom.php
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,20 @@ protected function _mergeNode(\DOMElement $node, $parentPath)
/* override node value */
if ($this->_isTextNode($node)) {
/* skip the case when the matched node has children, otherwise they get overridden */
if (!$matchedNode->hasChildNodes() || $this->_isTextNode($matchedNode)) {
if (!$matchedNode->hasChildNodes()
|| $this->_isTextNode($matchedNode)
|| $this->_isCdataNode($matchedNode)
) {
$matchedNode->nodeValue = $node->childNodes->item(0)->nodeValue;
}
} elseif ($this->_isCdataNode($node) && $this->_isTextNode($matchedNode)) {
/* Replace text node with CDATA section */
if ($this->_findCdataSection($node)) {
$matchedNode->nodeValue = $this->_findCdataSection($node)->nodeValue;
}
} elseif ($this->_isCdataNode($node) && $this->_isCdataNode($matchedNode)) {
/* Replace CDATA with new one */
$this->_replaceCdataNode($matchedNode, $node);
} else {
/* recursive merge for all child nodes */
foreach ($node->childNodes as $childNode) {
Expand Down Expand Up @@ -220,6 +231,56 @@ protected function _isTextNode($node)
return $node->childNodes->length == 1 && $node->childNodes->item(0) instanceof \DOMText;
}

/**
* Check if the node content is CDATA (probably surrounded with text nodes) or just text node
*
* @param \DOMNode $node
* @return bool
*/
protected function _isCdataNode($node)
{
// If every child node of current is NOT \DOMElement
// It is arbitrary combination of text nodes and CDATA sections.
foreach ($node->childNodes as $childNode) {
if ($childNode instanceof \DOMElement) {
return false;
}
}

return true;
}

/**
* Finds CDATA section from given node children
*
* @param \DOMNode $node
* @return \DOMCdataSection|null
*/
protected function _findCdataSection($node)
{
foreach ($node->childNodes as $childNode) {
if ($childNode instanceof \DOMCdataSection) {
return $childNode;
}
}
}

/**
* Replaces CDATA section in $oldNode with $newNode's
*
* @param \DOMNode $oldNode
* @param \DOMNode $newNode
*/
protected function _replaceCdataNode($oldNode, $newNode)
{
$oldCdata = $this->_findCdataSection($oldNode);
$newCdata = $this->_findCdataSection($newNode);

if ($oldCdata && $newCdata) {
$oldCdata->nodeValue = $newCdata->nodeValue;
}
}

/**
* Merges attributes of the merge node to the base node
*
Expand Down
31 changes: 31 additions & 0 deletions lib/internal/Magento/Framework/Config/Test/Unit/DomTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,37 @@ public function mergeDataProvider()
['override_node.xml', 'override_node_new.xml', [], null, 'override_node_merged.xml'],
['override_node_new.xml', 'override_node.xml', [], null, 'override_node_merged.xml'],
['text_node.xml', 'text_node_new.xml', [], null, 'text_node_merged.xml'],
'text node replaced with cdata' => [
'text_node_cdata.xml',
'text_node_cdata_new.xml',
[],
null,
'text_node_cdata_merged.xml'
],
'cdata' => ['cdata.xml', 'cdata_new.xml', [], null, 'cdata_merged.xml'],
'cdata with html' => ['cdata_html.xml', 'cdata_html_new.xml', [], null, 'cdata_html_merged.xml'],
'cdata replaced with text node' => [
'cdata_text.xml',
'cdata_text_new.xml',
[],
null,
'cdata_text_merged.xml'
],
'big cdata' => ['big_cdata.xml', 'big_cdata_new.xml', [], null, 'big_cdata_merged.xml'],
'big cdata with attribute' => [
'big_cdata_attribute.xml',
'big_cdata_attribute_new.xml',
[],
null,
'big_cdata_attribute_merged.xml'
],
'big cdata replaced with text' => [
'big_cdata_text.xml',
'big_cdata_text_new.xml',
[],
null,
'big_cdata_text_merged.xml'
],
[
'recursive.xml',
'recursive_new.xml',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0"?>
<!--
Copyright (c) Vaimo Group. All rights reserved.
See LICENSE_VAIMO.txt for license details.
-->
<root>
<node>
<subnode>
<![CDATA[Some Phrase]]>
</subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0"?>
<!--
Copyright (c) Vaimo Group. All rights reserved.
See LICENSE_VAIMO.txt for license details.
-->
<root>
<node>
<subnode attr="text">
<![CDATA[Some Phrase]]>
</subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0"?>
<!--
Copyright (c) Vaimo Group. All rights reserved.
See LICENSE_VAIMO.txt for license details.
-->
<root>
<node>
<subnode attr="text">
<![CDATA[Some Other Phrase]]>
</subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0"?>
<!--
Copyright (c) Vaimo Group. All rights reserved.
See LICENSE_VAIMO.txt for license details.
-->
<root>
<node>
<subnode>
<![CDATA[Some Other Phrase]]>
</subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0"?>
<!--
Copyright (c) Vaimo Group. All rights reserved.
See LICENSE_VAIMO.txt for license details.
-->
<root>
<node>
<subnode>
<![CDATA[Some Other Phrase]]>
</subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0"?>
<!--
Copyright (c) Vaimo Group. All rights reserved.
See LICENSE_VAIMO.txt for license details.
-->
<root>
<node>
<subnode>
<![CDATA[Some Other Phrase]]>
</subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0"?>
<!--
Copyright (c) Vaimo Group. All rights reserved.
See LICENSE_VAIMO.txt for license details.
-->
<root>
<node>
<subnode>
<![CDATA[Some Phrase]]>
</subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0"?>
<!--
Copyright (c) Vaimo Group. All rights reserved.
See LICENSE_VAIMO.txt for license details.
-->
<root>
<node>
<subnode>Some Other Phrase</subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0"?>
<!--
Copyright (c) Vaimo Group. All rights reserved.
See LICENSE_VAIMO.txt for license details.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, replace comments with corresponded Magento copyrights

-->
<root>
<node>
<subnode>Some Other Phrase</subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0"?>
<!--
Copyright (c) Vaimo Group. All rights reserved.
See LICENSE_VAIMO.txt for license details.
-->
<root>
<node>
<subnode><![CDATA[Some Phrase]]></subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0"?>
<!--
Copyright (c) Vaimo Group. All rights reserved.
See LICENSE_VAIMO.txt for license details.
-->
<root>
<node>
<subnode><![CDATA[Some <br /> Phrase]]></subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0"?>
<!--
Copyright (c) Vaimo Group. All rights reserved.
See LICENSE_VAIMO.txt for license details.
-->
<root>
<node>
<subnode><![CDATA[Some <br /> Other <br /> Phrase]]></subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0"?>
<!--
Copyright (c) Vaimo Group. All rights reserved.
See LICENSE_VAIMO.txt for license details.
-->
<root>
<node>
<subnode><![CDATA[Some <br /> Other <br /> Phrase]]></subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0"?>
<!--
Copyright (c) Vaimo Group. All rights reserved.
See LICENSE_VAIMO.txt for license details.
-->
<root>
<node>
<subnode><![CDATA[Some Other Phrase]]></subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0"?>
<!--
Copyright (c) Vaimo Group. All rights reserved.
See LICENSE_VAIMO.txt for license details.
-->
<root>
<node>
<subnode><![CDATA[Some Other Phrase]]></subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0"?>
<!--
Copyright (c) Vaimo Group. All rights reserved.
See LICENSE_VAIMO.txt for license details.
-->
<root>
<node>
<subnode><![CDATA[Some Phrase]]></subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0"?>
<!--
Copyright (c) Vaimo Group. All rights reserved.
See LICENSE_VAIMO.txt for license details.
-->
<root>
<node>
<subnode>Some Other Phrase</subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0"?>
<!--
Copyright (c) Vaimo Group. All rights reserved.
See LICENSE_VAIMO.txt for license details.
-->
<root>
<node>
<subnode>Some Other Phrase</subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0"?>
<!--
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
-->
<root>
<node>
<subnode>Some Phrase</subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?xml version="1.0"?>
<!--
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
-->
<root>
<node>
<subnode>
<![CDATA[Some Other Phrase]]>
</subnode>
</node>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?xml version="1.0"?>
<!--
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
-->
<root>
<node>
<subnode>
<![CDATA[Some Other Phrase]]>
</subnode>
</node>
</root>