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.

Sign up for GitHub

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

Jump to bottom

[Routing] support for array values in route defaults #11394

Merged
merged 1 commit into from Jun 23, 2016
Merged

[Routing] support for array values in route defaults #11394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/Symfony/Component/Routing/CHANGELOG.md
View file
Open in desktop
@@ -1,6 +1,11 @@
CHANGELOG
=========

3.2.0
-----

* Added support for `boolean`, `integer`, `float`, `string`, `list` and `map` defaults.

2.8.0
-----

Expand Down
103 changes: 102 additions & 1 deletion src/Symfony/Component/Routing/Loader/XmlFileLoader.php
View file
Open in desktop
Expand Up @@ -202,12 +202,16 @@ private function parseConfigs(\DOMElement $node, $path)
$condition = null;

foreach ($node->getElementsByTagNameNS(self::NAMESPACE_URI, '*') as $n) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Does anyone of you know if there was a special reason to use getElementsByTagNameNs() instead of iterating over the DOMNodeList in $node->childNodes?

Copy link
Member

Choose a reason for hiding this comment

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

to support namespaces

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't that covered by the elements from the childNodes too? It could then need some check for the namespaces afterwards but doesn't require to scan the complete subtree.

Copy link
Member

Choose a reason for hiding this comment

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

The idea was to ignore elements from another namespace and not throw an exception for unknown tag. So one could extend the loader and xsd to support custom elements, like in FosRestBundle.
I think this won't work with childNodes because it also returns element from other namespaces within the current node.

Copy link
Member Author

Choose a reason for hiding this comment

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

But throwing an exception contradicts this, doesn't it?

By the way, the new method just test for the elements local name. Should I add an additional check for the element's namespace?

Copy link
Member

Choose a reason for hiding this comment

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

only throw for unkown elements in same namespace

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, of course you're right. I updated the new code to also ignore elements with a different namespace.

if ($node !== $n->parentNode) {
continue;
}

switch ($n->localName) {
case 'default':
if ($this->isElementValueNull($n)) {
$defaults[$n->getAttribute('key')] = null;
} else {
$defaults[$n->getAttribute('key')] = trim($n->textContent);
$defaults[$n->getAttribute('key')] = $this->parseDefaultsConfig($n, $path);
}

break;
Expand All @@ -228,6 +232,103 @@ private function parseConfigs(\DOMElement $node, $path)
return array($defaults, $requirements, $options, $condition);
}

/**
* Parses the "default" elements.
*
* @param \DOMElement $element The "default" element to parse
* @param string $path Full path of the XML file being processed
*
* @return array|bool|float|int|string|null The parsed value of the "default" element
*/
private function parseDefaultsConfig(\DOMElement $element, $path)
{
if ($this->isElementValueNull($element)) {
return;
}

// Check for existing element nodes in the default element. There can
// only be a single element inside a default element. So this element
// (if one was found) can safely be returned.
foreach ($element->childNodes as $child) {
if (!$child instanceof \DOMElement) {
continue;
}

if (self::NAMESPACE_URI !== $child->namespaceURI) {
continue;
}

return $this->parseDefaultNode($child, $path);
}

// If the default element doesn't contain a nested "boolean", "integer",
// "float", "string", "list" or "map" element, the element contents will
// be treated as the string value of the associated default option.
return trim($element->textContent);
}

/**
* Recursively parses the value of a "default" element.
*
* @param \DOMElement $node The node value
* @param string $path Full path of the XML file being processed
*
* @return array|bool|float|int|string The parsed value
*
* @throws \InvalidArgumentException when the XML is invalid
*/
private function parseDefaultNode(\DOMElement $node, $path)
{
if ($this->isElementValueNull($node)) {
return;
}

switch ($node->localName) {
case 'boolean':
return 'true' === trim($node->nodeValue) || '1' === trim($node->nodeValue);
case 'integer':
return (int) trim($node->nodeValue);
case 'float':
return (float) trim($node->nodeValue);
case 'string':
return trim($node->nodeValue);
case 'list':
$list = array();

foreach ($node->childNodes as $element) {
if (!$element instanceof \DOMElement) {
continue;
}

if (self::NAMESPACE_URI !== $element->namespaceURI) {
continue;
}

$list[] = $this->parseDefaultNode($element, $path);
}

return $list;
case 'map':
$map = array();

foreach ($node->childNodes as $element) {
if (!$element instanceof \DOMElement) {
continue;
}

if (self::NAMESPACE_URI !== $element->namespaceURI) {
continue;
}

$map[$element->getAttribute('key')] = $this->parseDefaultNode($element, $path);
Copy link
Member

Choose a reason for hiding this comment

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

you should check that the key attribute is set

Copy link
Member Author

Choose a reason for hiding this comment

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

The key attribute for the map elements is mandatory.

Copy link
Member

Choose a reason for hiding this comment

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

That's even a better reason to check it, isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

But we use the schema definition to validate the XML before.

Copy link
Member

Choose a reason for hiding this comment

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

yeah, I forgot that the XSD already checked it

}

return $map;
default:
throw new \InvalidArgumentException(sprintf('Unknown tag "%s" used in file "%s". Expected "boolean", "integer", "float", "string", "list" or "map".', $node->localName, $path));
}
}

private function isElementValueNull(\DOMElement $element)
{
$namespaceUri = 'http://www.w3.org/2001/XMLSchema-instance';
Expand Down
84 changes: 83 additions & 1 deletion src/Symfony/Component/Routing/Loader/schema/routing/routing-1.0.xsd
View file
Open in desktop
Expand Up @@ -26,7 +26,7 @@

<xsd:group name="configs">
<xsd:choice>
<xsd:element name="default" nillable="true" type="element" />
<xsd:element name="default" nillable="true" type="default" />
<xsd:element name="requirement" type="element" />
Copy link
Member

Choose a reason for hiding this comment

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

could you also add the required "key" attribute for "requirement" and "option" to the xsd? this does seem to be there yet. but xsd seems to be easy for you :)

Copy link
Member Author

Choose a reason for hiding this comment

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

This is already be covered by the element definition in the XSD, isn't it?

Copy link
Member

Choose a reason for hiding this comment

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

Oh true, didn't see that.

<xsd:element name="option" type="element" />
<xsd:element name="condition" type="xsd:string" />
Expand Down Expand Up @@ -54,11 +54,93 @@
<xsd:attribute name="methods" type="xsd:string" />
</xsd:complexType>

<xsd:complexType name="default" mixed="true">
<xsd:choice minOccurs="0" maxOccurs="1">
<xsd:element name="boolean" type="xsd:boolean" />
<xsd:element name="integer" type="xsd:integer" />
<xsd:element name="float" type="xsd:float" />
<xsd:element name="string" type="xsd:string" />
<xsd:element name="list" type="list" />
<xsd:element name="map" type="map" />
</xsd:choice>
<xsd:attribute name="key" type="xsd:string" use="required" />
</xsd:complexType>

<xsd:complexType name="element">
<xsd:simpleContent>
<xsd:extension base="xsd:string">
<xsd:attribute name="key" type="xsd:string" use="required" />
</xsd:extension>
</xsd:simpleContent>
</xsd:complexType>

<xsd:complexType name="list">
<xsd:choice minOccurs="0" maxOccurs="unbounded">
<xsd:element name="boolean" nillable="true" type="xsd:boolean" />
<xsd:element name="integer" nillable="true" type="xsd:integer" />
<xsd:element name="float" nillable="true" type="xsd:float" />
<xsd:element name="string" nillable="true" type="xsd:string" />
<xsd:element name="list" nillable="true" type="list" />
<xsd:element name="map" nillable="true" type="map" />
</xsd:choice>
</xsd:complexType>

<xsd:complexType name="map">
<xsd:choice minOccurs="0" maxOccurs="unbounded">
<xsd:element name="boolean" nillable="true" type="map-boolean-entry" />
<xsd:element name="integer" nillable="true" type="map-integer-entry" />
<xsd:element name="float" nillable="true" type="map-float-entry" />
<xsd:element name="string" nillable="true" type="map-string-entry" />
<xsd:element name="list" nillable="true" type="map-list-entry" />
<xsd:element name="map" nillable="true" type="map-map-entry" />
</xsd:choice>
</xsd:complexType>

<xsd:complexType name="map-boolean-entry">
<xsd:simpleContent>
<xsd:extension base="xsd:boolean">
<xsd:attribute name="key" type="xsd:string" use="required" />
</xsd:extension>
</xsd:simpleContent>
</xsd:complexType>

<xsd:complexType name="map-integer-entry">
<xsd:simpleContent>
<xsd:extension base="xsd:integer">
<xsd:attribute name="key" type="xsd:string" use="required" />
</xsd:extension>
</xsd:simpleContent>
</xsd:complexType>

<xsd:complexType name="map-float-entry">
<xsd:simpleContent>
<xsd:extension base="xsd:float">
<xsd:attribute name="key" type="xsd:string" use="required" />
</xsd:extension>
</xsd:simpleContent>
</xsd:complexType>

<xsd:complexType name="map-string-entry">
<xsd:simpleContent>
<xsd:extension base="xsd:string">
<xsd:attribute name="key" type="xsd:string" use="required" />
</xsd:extension>
</xsd:simpleContent>
</xsd:complexType>

<xsd:complexType name="map-list-entry">
<xsd:complexContent>
<xsd:extension base="list">
<xsd:attribute name="key" type="xsd:string" use="required" />
</xsd:extension>
</xsd:complexContent>
</xsd:complexType>

<xsd:complexType name="map-map-entry">
<xsd:complexContent>
<xsd:extension base="map">
<xsd:attribute name="key" type="xsd:string" use="required" />
</xsd:extension>
</xsd:complexContent>
</xsd:complexType>
</xsd:schema>
20 changes: 20 additions & 0 deletions src/Symfony/Component/Routing/Tests/Fixtures/list_defaults.xml
View file
Open in desktop
@@ -0,0 +1,20 @@
<?xml version="1.0" encoding="UTF-8" ?>
<routes xmlns="http://symfony.com/schema/routing"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/routing
http://symfony.com/schema/routing/routing-1.0.xsd">

<route id="blog" path="/blog">
<default key="_controller">
<string>AcmeBlogBundle:Blog:index</string>
</default>
<default key="values">
<list>
<boolean>true</boolean>
<integer>1</integer>
<float>3.5</float>
<string>foo</string>
</list>
</default>
</route>
</routes>
22 changes: 22 additions & 0 deletions src/Symfony/Component/Routing/Tests/Fixtures/list_in_list_defaults.xml
View file
Open in desktop
@@ -0,0 +1,22 @@
<?xml version="1.0" encoding="UTF-8" ?>
<routes xmlns="http://symfony.com/schema/routing"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/routing
http://symfony.com/schema/routing/routing-1.0.xsd">

<route id="blog" path="/blog">
<default key="_controller">
<string>AcmeBlogBundle:Blog:index</string>
</default>
<default key="values">
<list>
<list>
<boolean>true</boolean>
<integer>1</integer>
<float>3.5</float>
<string>foo</string>
</list>
</list>
</default>
</route>
</routes>
22 changes: 22 additions & 0 deletions src/Symfony/Component/Routing/Tests/Fixtures/list_in_map_defaults.xml
View file
Open in desktop
@@ -0,0 +1,22 @@
<?xml version="1.0" encoding="UTF-8" ?>
<routes xmlns="http://symfony.com/schema/routing"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/routing
http://symfony.com/schema/routing/routing-1.0.xsd">

<route id="blog" path="/blog">
<default key="_controller">
<string>AcmeBlogBundle:Blog:index</string>
</default>
<default key="values">
<map>
<list key="list">
<boolean>true</boolean>
<integer>1</integer>
<float>3.5</float>
<string>foo</string>
</list>
</map>
</default>
</route>
</routes>
22 changes: 22 additions & 0 deletions src/Symfony/Component/Routing/Tests/Fixtures/list_null_values.xml
View file
Open in desktop
@@ -0,0 +1,22 @@
<?xml version="1.0" encoding="UTF-8" ?>
<routes xmlns="http://symfony.com/schema/routing"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/routing
http://symfony.com/schema/routing/routing-1.0.xsd">

<route id="blog" path="/blog">
<default key="_controller">
<string>AcmeBlogBundle:Blog:index</string>
</default>
<default key="list">
<list>
<boolean xsi:nil="true" />
<integer xsi:nil="true" />
<float xsi:nil="1" />
<string xsi:nil="true" />
<list xsi:nil="true" />
<map xsi:nil="true" />
</list>
</default>
</route>
</routes>
20 changes: 20 additions & 0 deletions src/Symfony/Component/Routing/Tests/Fixtures/map_defaults.xml
View file
Open in desktop
@@ -0,0 +1,20 @@
<?xml version="1.0" encoding="UTF-8" ?>
<routes xmlns="http://symfony.com/schema/routing"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/routing
http://symfony.com/schema/routing/routing-1.0.xsd">

<route id="blog" path="/blog">
<default key="_controller">
<string>AcmeBlogBundle:Blog:index</string>
</default>
<default key="values">
<map>
<boolean key="public">true</boolean>
<integer key="page">1</integer>
<float key="price">3.5</float>
<string key="title">foo</string>
</map>
</default>
</route>
</routes>
22 changes: 22 additions & 0 deletions src/Symfony/Component/Routing/Tests/Fixtures/map_in_list_defaults.xml
View file
Open in desktop
@@ -0,0 +1,22 @@
<?xml version="1.0" encoding="UTF-8" ?>
<routes xmlns="http://symfony.com/schema/routing"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/routing
http://symfony.com/schema/routing/routing-1.0.xsd">

<route id="blog" path="/blog">
<default key="_controller">
<string>AcmeBlogBundle:Blog:index</string>
</default>
<default key="values">
<list>
<map>
<boolean key="public">true</boolean>
<integer key="page">1</integer>
<float key="price">3.5</float>
<string key="title">foo</string>
</map>
</list>
</default>
</route>
</routes>
22 changes: 22 additions & 0 deletions src/Symfony/Component/Routing/Tests/Fixtures/map_in_map_defaults.xml
View file
Open in desktop
@@ -0,0 +1,22 @@
<?xml version="1.0" encoding="UTF-8" ?>
<routes xmlns="http://symfony.com/schema/routing"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/routing
http://symfony.com/schema/routing/routing-1.0.xsd">

<route id="blog" path="/blog">
<default key="_controller">
<string>AcmeBlogBundle:Blog:index</string>
</default>
<default key="values">
<map>
<map key="map">
<boolean key="public">true</boolean>
<integer key="page">1</integer>
<float key="price">3.5</float>
<string key="title">foo</string>
</map>
</map>
</default>
</route>
</routes>