Customize the name of the root XML element

[bajtos]

At the moment, when producing a response in the XML format, strong-remoting always wrap the response body in <response> element (source code).

We should allow developers to customise the name of this root element, for example to produce a response body like this:

<foo>bar</foo>

A possible solution is to implement a new remoting setting at the shared-method level:

var fn = function(cb) {
  cb(null, 'bar');
};

setupRemoting(fn, {
  "accepts": [],
  "returns": { "type": "string", "root": true },
  "xml": { "rootElement": "myCustomName" } }
});

[pulkitsinghal]

That sounds good and what is your opinion if I suggest: "Let's not try to wrap it at all, since the user has already set root:true which means they've got a handle on it and don't need the framework to wrap anything? It should let it flow through as-is."

[bajtos]

@pulkitsinghal That sounds good and what is your opinion if I suggest: "Let's not try to wrap it at all, since the user has already set root:true which means they've got a handle on it and don't need the framework to wrap anything? It should let it flow through as-is."

I'm afraid that's not possible. Consider the following method:

function(cb) {
  cb(null, { a:1, b:2 });
}
// cb arg1 is { type: Object, root: true }

In JSON, the object is serialized as follows:

{ 
  "a": 1,
  "b": 2,
}

However, there is no way how to do the same in XML, the following is not a well-formed document:

<a>1</a>
<b>2</b>

That's why we need a wrapper element.

[pulkitsinghal]

@bajtos - Ok I was making the case that as a developer who has set: root: true, I would be responsible for returning my data as:

function(cb) {
  cb(null, { rootElem: {a:1, b:2 }} );
}

[pulkitsinghal]

Whatever gets a solution sooner, I won't argue :)

[bajtos]

I was making the case that as a developer who has set: root: true, I would be responsible for returning my data as:

function(cb) {
  cb(null, { rootElem: {a:1, b:2 }} );
}

Makes sense. It is what I was initially thinking about too. However, to preserve backwards compatibility, we would have to add a new option for that, e.g.

returns: { type: 'object', root: true, xml: { wrapperElement: false } }

Implementation wise, I think it will be easier for us to simply change the name of the wrapper element depending on the configuration, rather than detecting whether the returned root object has a single property.

[pulkitsinghal]

@bajtos - Agreed!

If you want me to make code changes and submit PR ... would you mind pointing me to where to start making these changes? I'm often hesitant to submit PRs because in my opinion, the PRs could get stuck when asked to add an unit test ... as I don't really know how or where to do it within loopback.

[bajtos]

@pulkitsinghal I asked @davidcheung to implement this for you. But if you don't mind to contribute the patch yourself, then that would be even better!

The hard-coded 'response' element name is here:

strong-remoting/lib/http-context.js

Line 347 in df59c64

xml = js2xmlparser('response', input, {

I found only one unit-test for XML:

strong-remoting/test/rest.test.js

Lines 258 to 282 in c43b8ed

	it('should enable XML types via `options.rest.xml`', function(done) {
	objects.options.rest = { xml: true };
	var method = givenSharedStaticMethod(
	function(value, cb) { cb(null, { key: value }); },
	{
	accepts: { arg: 'value', type: 'string' },
	returns: { arg: 'result', type: 'object' }
	});
	request(app).post(method.url)
	.set('Accept', ACCEPT_XML_OR_ANY)
	.set('Content-Type', 'application/json')
	.send({ value: 'some-value' })
	.expect(200)
	.expect('Content-Type', /xml/)
	.end(function(err, res) {
	if (err) return done(err);
	expect(res.text.replace(/>\s+</mg, '><')).to.equal(
	'<?xml version="1.0" encoding="UTF-8"?>' +
	'<response><result><key>some-value</key></result></response>'
	);
	done();
	});
	});