This project is archived and is in readonly mode.

#3243 ✓stale
Nick M

[PATCH] to_xml doesn't support nested structures and other inconstancies

Reported by Nick M | September 22nd, 2009 @ 05:40 AM

I've ran into a few problems using to_xml.

  • On an ActiveRecord model, calling to_xml(:methods => :custom_method) where custom_method returns a hash, the hash is simply converted to a string and not represented in XML.
  • In the same situation, if custom_method returns an array of values, the array is mashed together as a single string.
  • to_xml(:camelize => true) doesn't always play nice with arrays. The inner tags might be camelized, but the outer tag never seems to be properly camelized.

These are just a few problems, but more generally I've repeatedly run into problems where to_json can simply handle converting any object to JSON, but to_xml will balk, convert objects into not so useful strings, or simply not do what's expected.

I began trying to fix these problems, but quickly fell down a rabbit-hole of completely refactoring how to_xml works. So now I have a patch where to_xml is organized and works much like to_json does now. It fixes all of these problems with certain objects not supporting to_xml by allowing any object to define how it will be coerced into an XML value and how it will build its XML tags if necessary.

Here's a quick example showing the previous problems and how things work with the patch, which I think would be the expected behavior:

class Report < ActiveRecord::Base
  def item_ids
    [8, 9]
  end

  def metadata
    { :color => "blue", :options => [:blue, :red] }
  end
end

report.to_xml(:methods => [:item_ids, :metadata])

Previously, this would generate:

<report>
  ...
  <item-ids>89</item-ids>
  <metadata>colorblueoptionsbluered</metadata>
</report>

Now it generates:

<report>
  ...
  <item-ids type="array">
    <item-id type="integer">8</item-id>
    <item-id type="integer">9</item-id>
  </item-ids>
  <metadata>
    <color>blue</color>
    <options type="array">
      <option type="symbol">blue</option>
      <option type="symbol">red</option>
    </options>
  </metadata>
</report>

I've introduced new tests to test these new cases. The patch maintains backwards compatibility with all previous tests. The one exception is a newer ActiveModel test where a method returns a hash and it was expecting to be converted to YAML. ActiveRecord serialized_attributes are still converted to YAML, but since ActiveModel doesn't know about serialized_attributes, the hash returned there is converted into nested XML.

Any feedback or questions are welcome. Hopefully this patch makes to_xml behave more as expected with other data types, and also makes it more flexible to customize how objects respond to XML encoding. The patch provided is against the latest master, but if this patch seems desirable, is there any chance of getting these changes implemented on 2.3 stable? I started out patching 2-3-stable, so I have most of this working there, but since this patch grew a bit more than I expected, I thought I'd get feedback first. But I can provide a 2.3 patch if it's wanted.

Here are a couple of older bug reports documenting some of these same issues: #458 and #1576

Comments and changes to this ticket

  • CancelProfileIsBroken

    CancelProfileIsBroken September 25th, 2009 @ 11:55 AM

    • Tag changed from activerecord, bug, patch, serialization, to_xml, xml to activerecord, bug, bugmash, patch, serialization, to_xml, xml
  • Elomar França

    Elomar França September 28th, 2009 @ 12:40 AM

    1. Agree with the fixes, but the patch doesn't apply neither on master nor on 2-3-stable.
  • Nick M

    Nick M September 28th, 2009 @ 01:54 AM

    I think this updated patch should apply to master again. Let me know if there are still problems with it.

    I can bring this same functionality back to 2-3-stable, but things are organized a bit differently there, so I'd just have to spend a bit of time sorting all that out and creating a separate patch. I think it would be great if these changes could be applied to 2.3 (they should be completely backwards compatible with what used to work correctly), but I wanted to see if others agreed before producing another patch for 2.3.

  • Rizwan Reza

    Rizwan Reza February 12th, 2010 @ 12:46 PM

    • Tag changed from activerecord, bug, bugmash, patch, serialization, to_xml, xml to activerecord, bug, patch, serialization, to_xml, xml
  • Christos Trochalakis

    Christos Trochalakis May 14th, 2010 @ 01:50 PM

    We are building a json/xml api and the xml output is broken due to this bug, we would love to see this pushed to both master and 2.3 branch. JSON doesn't have those issues due to the as_json propagation, so implementing as_xml method seems like the right thing to do.

    ps: the patch has become outdated

  • Neeraj Singh

    Neeraj Singh May 14th, 2010 @ 08:38 PM

    @Christos Trochalakis Can you please elaborate what part of xml output you don't like in the give below scenario.

    ActiveRecord::Schema.define(:version => 20100514192542) do
    
      create_table "reports", :force => true do |t|
        t.string   "name"
        t.datetime "created_at"
        t.datetime "updated_at"
      end
    
    end
    class Report < ActiveRecord::Base
      def item_ids
        [8,9]
      end
      def metadata
        {:color => 'blue', :options => [:blue, :red]}
      end
    
      def self.lab
        Report.first.to_xml(:methods => [:item_ids, :metadata])
      end
    end
    #rails -v : Rails 3.0.0.beta3
    #ruby -v : ruby 1.8.7 (2010-01-10 patchlevel 249) [i686-darwin10.3.0]
    

    Report.lab comes out to be

    <name>GDP growth</name>
    <created-at type="datetime">2010-05-14T19:29:20Z</created-at>
    <updated-at type="datetime">2010-05-14T19:29:20Z</updated-at>
    <id type="integer">1</id>
    <item-ids type="array">
      <item-id type="integer">8</item-id>
      <item-id type="integer">9</item-id>
    </item-ids>
    <metadata>
      <color>blue</color>
      <options type="array">
        <option type="symbol">blue</option>
        <option type="symbol">red</option>
      </options>
    </metadata>
    

    The xml looks good to me.

  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:39 PM

    • State changed from “new” to “open”
    • Importance changed from “” to “”

    This issue has been automatically marked as stale because it has not been commented on for at least three months.

    The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

    Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:39 PM

    • State changed from “open” to “stale”

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Attachments

Pages