Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

XmlView: configure top level element name #934

Closed
wants to merge 5 commits into from

5 participants

@larryb82

http://api20.cakephp.org/class/xml-view

XmlView always uses 'request' as the top level element name. This change allows it to be configured by setting XmlView.RootNodeName.

Configure::write('XmlView.RootNodeName', 'custom_name');
@larryb82 larryb82 XmlView: configure top level element name
Allow the top level element name to be configured by setting
XmlView.RootNodeName.
54056fd
lib/Cake/Test/Case/View/XmlViewTest.php
@@ -35,6 +35,14 @@ class XmlViewTest extends CakeTestCase {
* @return void
*/
public function testRenderWithoutView() {
+ Configure::write('XmlView.RootNodeName', null);
@markstory Owner

Why a configure variable? Why not use a viewVariable? This makes fewer dependencies on Configure and I think will make things simpler overall. Perhaps something like _rootNode

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lib/Cake/Test/Case/View/XmlViewTest.php
@@ -35,6 +35,14 @@ class XmlViewTest extends CakeTestCase {
* @return void
*/
public function testRenderWithoutView() {
+ Configure::write('XmlView.RootNodeName', null);
+ $this->__testRenderWithoutView('response');
+
+ Configure::write('XmlView.RootNodeName', 'custom_name');
+ $this->__testRenderWithoutView('custom_name');
+ }
+
+ private function __testRenderWithoutView($rootNodeName) {
@markstory Owner

We generally avoid private methods, favoring protected ones instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lib/Cake/View/XmlView.php
@@ -99,15 +99,18 @@ public function render($view = null, $layout = null) {
* @return string The serialized data
*/
protected function _serialize($serialize) {
+ $root = Configure::read('XmlView.RootNodeName');
@ADmad Collaborator
ADmad added a note

That's a gross misuse of the Configure class. A proper way to achieve this for eg. would be setting a special view variable named _rootElement to specify alternate root element name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dereuromark
Collaborator
"XmlView always uses 'request'..."

you mean response, i guess.
I also like the basic idea behind it.

@larryb82

How about:

$root = isset($this->viewVars['_rootElement']) ? $this->viewVars['_rootElement'] : 'response';

I originally used Configure because I wanted the change to be application wide. With this approach, placing the the following in your AppController would have the same effect.

public function beforeRender() {
    parent::beforeRender();
    $this->set('_rootElement', 'custom_name');
}

I can update the tests and submit the changes if you think it's worthwhile.

Also, is there a problem with the way I used private functions in the test?

@ADmad
Collaborator

_rootNode like markstory suggested would probably be a better name for that special variable.

As for the private function, change it to protected.

@larryb82 larryb82 XmlView: configure top level element name
Allows the root element of the xml output to be changed by setting the
view variable '_rootElement'.
d623d9e
@markstory markstory commented on the diff
lib/Cake/Test/Case/View/XmlViewTest.php
@@ -64,6 +64,14 @@ public function testRenderWithoutView() {
$expected = Xml::build(array('response' => array('users' => $data)))->asXML();
$this->assertSame($expected, $output);
+
+
@markstory Owner

Little nitpick, you should only have 1 newline here.

@ADmad Collaborator
ADmad added a note

Better not give the code sniffer reasons to complain :smile:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@markstory markstory commented on the diff
lib/Cake/Test/Case/View/XmlViewTest.php
((7 lines not shown))
$expected = array(
'response' => array('no' => $data['no'], 'user' => $data['user'])
);
$this->assertSame(Xml::build($expected)->asXML(), $output);
- $this->assertSame('application/xml', $Response->type());
+
+
@markstory Owner

Little nitpick, you should only have 1 newline here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@markstory
Owner

@ADmad I'm fine with either _rootElement or _rootNode, both fit the lingo of XML. rootNode is a bit shorter to type though, so a minor advantage there.

lib/Cake/View/XmlView.php
@@ -99,15 +99,17 @@ public function render($view = null, $layout = null) {
* @return string The serialized data
*/
protected function _serialize($serialize) {
+ $root = isset($this->viewVars['_rootElement']) ? $this->viewVars['_rootElement'] : 'response';
@ADmad Collaborator
ADmad added a note

I would still prefer the view var name _rootNode, less typing :smile:. Also better to name the local var $rootNode instead of $root to match the view var.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@larryb82 larryb82 XmlView: renamed _rootElement to _rootNode
Local variable in _serialize renamed as well.
1ab8f98
@lorenzo
Owner

except for the two extra lines in the diff, it looks good to me

@ADmad
Collaborator

@larryb82 Please remove the extra new lines too and squash the commits and this will be ready for merging.

larryb82 added some commits
@larryb82 larryb82 XmlView: configure top level element name
Allows the root element of the xml output to be changed by setting the
view variable '_rootNode'.
02c1b49
@larryb82 larryb82 Merge branch '2.3' of https://github.com/larryb82/cakephp into 2.3
Conflicts:
	lib/Cake/Test/Case/View/XmlViewTest.php
c3af467
@larryb82

Sorry about the double commit. The squash tripped me up a bit. I'm more of an SVN guy. 02c1b49 looks fine to me.

@ADmad
Collaborator

There are now 5 commits in this PR :)

@larryb82

@ADmad, hope I didn't make this too hard on you guys. Like I said, 02c1b49 is the commit that accounts for all the feedback. Let me know if you need anything else.

@markstory
Owner

I'll squash and merge into 2.3

@markstory markstory was assigned
@markstory
Owner

Squashed and merged in a60a730

@markstory markstory closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 2, 2012
  1. @larryb82

    XmlView: configure top level element name

    larryb82 authored
    Allow the top level element name to be configured by setting
    XmlView.RootNodeName.
  2. @larryb82

    XmlView: configure top level element name

    larryb82 authored
    Allows the root element of the xml output to be changed by setting the
    view variable '_rootElement'.
  3. @larryb82

    XmlView: renamed _rootElement to _rootNode

    larryb82 authored
    Local variable in _serialize renamed as well.
Commits on Nov 3, 2012
  1. @larryb82

    XmlView: configure top level element name

    larryb82 authored
    Allows the root element of the xml output to be changed by setting the
    view variable '_rootNode'.
  2. @larryb82

    Merge branch '2.3' of https://github.com/larryb82/cakephp into 2.3

    larryb82 authored
    Conflicts:
    	lib/Cake/Test/Case/View/XmlViewTest.php
This page is out of date. Refresh to see the latest.
View
18 lib/Cake/Test/Case/View/XmlViewTest.php
@@ -64,6 +64,13 @@ public function testRenderWithoutView() {
$expected = Xml::build(array('response' => array('users' => $data)))->asXML();
$this->assertSame($expected, $output);
+
+ $Controller->set('_rootNode', 'custom_name');
@markstory Owner

Little nitpick, you should only have 1 newline here.

@ADmad Collaborator
ADmad added a note

Better not give the code sniffer reasons to complain :smile:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $View = new XmlView($Controller);
+ $output = $View->render(false);
+
+ $expected = Xml::build(array('custom_name' => array('users' => $data)))->asXML();
+ $this->assertSame($expected, $output);
}
/**
@@ -79,13 +86,20 @@ public function testRenderWithoutViewMultiple() {
$Controller->set($data);
$Controller->set('_serialize', array('no', 'user'));
$View = new XmlView($Controller);
+ $this->assertSame('application/xml', $Response->type());
$output = $View->render(false);
-
$expected = array(
'response' => array('no' => $data['no'], 'user' => $data['user'])
);
$this->assertSame(Xml::build($expected)->asXML(), $output);
- $this->assertSame('application/xml', $Response->type());
+
+ $Controller->set('_rootNode', 'custom_name');
@markstory Owner

Little nitpick, you should only have 1 newline here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $View = new XmlView($Controller);
+ $output = $View->render(false);
+ $expected = array(
+ 'custom_name' => array('no' => $data['no'], 'user' => $data['user'])
+ );
+ $this->assertSame(Xml::build($expected)->asXML(), $output);
}
/**
View
8 lib/Cake/View/XmlView.php
@@ -99,15 +99,17 @@ public function render($view = null, $layout = null) {
* @return string The serialized data
*/
protected function _serialize($serialize) {
+ $rootNode = isset($this->viewVars['_rootNode']) ? $this->viewVars['_rootNode'] : 'response';
+
if (is_array($serialize)) {
- $data = array('response' => array());
+ $data = array($rootNode => array());
foreach ($serialize as $key) {
- $data['response'][$key] = $this->viewVars[$key];
+ $data[$rootNode][$key] = $this->viewVars[$key];
}
} else {
$data = isset($this->viewVars[$serialize]) ? $this->viewVars[$serialize] : null;
if (is_array($data) && Set::numeric(array_keys($data))) {
- $data = array('response' => array($serialize => $data));
+ $data = array($rootNode => array($serialize => $data));
}
}
return Xml::fromArray($data)->asXML();
Something went wrong with that request. Please try again.