record.to_xml(:methods => other_ids) yields incorrect representation

Oops, didn't mean to change the tags - that just happened ... (but I guess it's right that way anyways)

Hi Clemens, thanks for the comment. When it comes to type conversion, Rails certainly uses this to elegant effect in the JSON class.

However, the problem is that this is not a design issue (as applied to type conversion or content types). It's a data issue.

In other words, JSON and XML are merely data formats. The data itself should not change. In this case it does.

Rails to_json gets it right (Ruby array -> Javascript array). Rails to_xml gets it wrong (Ruby array -> concatenated string). to_xml gets resource collection arrays right. I see no reason why it can't get this right.

But regardless, it's a problem because :methods => other_ids is in the Rails docs and is supposed to be able to handle this. And to prove the point, the newly revamped Rails 2.1 to_json (which added :include and :methods support as far as I know) can handle this.

So it's actually quite a discrepancy. To resolve it, we either need to force to_json to concatenate any :methods => other_ids arrays to strings as to_xml does (which would serve no utilitarian purpose). Or update to_xml to the level that to_json is at in this regard.

Re: using :include vs :methods, I do use :include in other resources, but not for many-to-many resources where it makes more sense to use id references, as these kinds of relationships can often be of a more categorical/static/lookup nature.

In any event, the nature of REST is atomicity (i.e. smaller, simpler, lighter, factorized resources that can be cached and combined more easily with others in different ways as opposed to large data clumps). :methods is far closer to this ideal than :include.

It's important we get this fixed.

Yep, still a problem.

Including the result of an other_ids method for JSON gives an array eg. [1,2,3,4,5]

Including the result of the same other_ids method for XML gives a string eg. "12345"

I'm not yet familiar enough with the workings of ActiveRecord to provide any form of patch.

As Clements mentioned earlier any data type that is not in the mentioned list will be treated as string. Two major data types that are causing problems are hash and array.

A recap of the issue.

class User
def lab

["a","b"]

end def lab2

{:foo => 'bar'}

end end

User.first.to_xml(:methods => [:lab, :lab2])
abc
foobar

User.first.to_json(:methods => [:lab, :lab2])
{lab {}lab2

This issue exists in both 2.3.5 branch and rails3 edge.

I understand that in rails community mostly people use json but "to_xml" should be as close to "to_json" as much possible.

lighthouse app team:

Please put preview next to "Update Ticket". Even better would be a "live preview".

With that above patch the output would be like this for hash and array instead of their to_s value.


{'john' : 'smith','foo' : 'bar'}
['a','b','c']


"hash.to_xml already works"

Can you elaborate on that?

#on 2-3-edge
h = {:foo => 'bar'}
=> {:foo=>"bar"}
h.to_xml
=> "<hash>\n  <foo>bar</foo>\n</hash>\n"

hash.to_xml prints to_s value. However the values that are printed for hash when model.to_json is called is entirely different.

Take a look at this case.

class User
 def lab
   %w( a b c)
 end
 def lab2
  {:foo => 'bar'
 end
end

# current 2-3-edge
User.first.to_xml(:methods => [:lab, :lab2])
<user>
 <id type="integer">1</id>
 <name>john</name>
 <lab>abc</lab>
 <lab2>foobar</lab2>
</user>

# current 2-3-edge
User.first.to_json(:methods => [:lab, :lab2])
"{"user":{"lab2":{"foo":"bar"},"name":"john","id":1,"lab":["a","b","c"]}}"

I think the value of to_xml for hash should match with the to_json value as much as possible.

Thanks for quick update.

Am I missing something here?

I cannot tell Rails 2.3 behavior, but on Rails 3.0:

{ :a => { :b => :c} }.to_xml #=>
  "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<hash>\n  <a>\n    <b type=\"symbol\">c</b>\n  </a>\n</hash>\n"

This is the behavior I expect when serializing a hash. It should not match JSON. It should match XML. The same for an array.

Got it. I see what you mean. Thanks.

In that case I will first work on a patch for rails3 and will later work on 2-3 branch.

Hey, that's nice! We just need to wrap a few loose ends:

1) Why you removed the yaml test?

2) You should not handle the hash in a conditional inside "add_attributes". Try to use just XML_FORMATTING;

3) The array formatting should output something like this:

  [1,2,3].to_xml #=> <socials type="array"><social>1</social><social>2</social><social>3</social></socials>

4) And we should add XML_PARSERS, to ensure that arrays and hash can be parsed back.

Good work!

Thanks for the quick feedback. Will send you a new patch soon.

Attached is a new patch.

1) yaml test has been restored.

2) For the "array" and "hash" data type the content is prebuilt and adding the content to the builder using tag! method escapes all the content. The only way to ensure that content is not escaped is to use "<<" and I could not find a better place to put the logic. If you find a better place where the logic should be shifted then do let me know.

3) Array behaves as you suggested.

4) Completely missed your 4th point when I read your message in the morning. Have not worked on implementing same features in XML_PARSERS. Will wait for your feedback on the work done so far. If everything looks good then I will work on parsing the content back.

Hey, just want to let you know that I'm discussing your patch with Jeremy Kemper and I hope to have feedback soon.

Thanks for the detailed feedback.

I should have checked if array respond to to_xml or not. Well now I know. :-)

As you suggested, I will take baby steps and will create incremental patches. Will ping you in the mailing list if I have any concerns.

Thanks again for the detailed feedback.

No code was required to fix this issue since all the work is now being done by to_xml itself. Look at this ticket for more information.

https://rails.lighthouseapp.com/projects/8994/tickets/4490

Attached is a patch which covers some test cases. Patch is against rails master.

Automatic cleanup of spam.

louis vuitton watches