Skip to content

Rails 3.2.8 ActiveModel::Serializers::Xml::Serializer ignores has_one sti #7471

Closed
cliochris opened this Issue Aug 28, 2012 · 7 comments

3 participants

@cliochris

I initially reported this bug against Rails 2.3.10 on February 10, 2011, see my report at https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/6406-activerecordxmlserializer-fails-with-has_one-types.

This bug remains unfixed in Rails 3.2.8. To see the problem, run the following code:

class Container < ActiveRecord::Base
  has_many :sti_outer_objs

  def dump
    to_xml({:include=>{:sti_outer_objs=>{:include=>{:sti_inner_obj=>{}}}}})
  end
end

class StiOuterObj < ActiveRecord::Base
  belongs_to :container
  has_one :sti_inner_obj
end

class StiOuterDerivedObj < StiOuterObj
end

class StiInnerObj < ActiveRecord::Base
  belongs_to :sti_outer_obj
end

class StiInnerDerivedObj < StiInnerObj
end

class Setup < ActiveRecord::Migration
  def self.up
    create_table :containers do |t|
    end
    create_table :sti_outer_objs do |t|
      t.string :type
      t.belongs_to :container
    end
    create_table :sti_inner_objs do |t|
      t.string :type
      t.belongs_to :sti_outer_obj
    end
    container = Container.create!
    sti_outer_derived = StiOuterDerivedObj.create!(:container => container)
    sti_inner_derived = StiInnerDerivedObj.create!(:sti_outer_obj => sti_outer_derived)
    container = Container.find(container.id)
    puts container.dump
  end

  def self.down
    drop_table :containers
    drop_table :sti_outer_objs
    drop_table :sti_inner_objs
  end
end

Expected output:

<?xml version="1.0" encoding="UTF-8"?>
<container>
  <id type="integer">1</id>
  <sti-outer-objs type="array">
    <sti-outer-obj type="StiOuterDerivedObj">
      <id type="integer">1</id>
      <container-id type="integer">1</container-id>
      <sti-inner-obj type="StiInnerDerivedObj">
        <id type="integer">1</id>
        <sti-outer-obj-id type="integer">1</sti-outer-obj-id>
      </sti-inner-obj>
    </sti-outer-obj>
  </sti-outer-objs>
</container>

Actual output:

<?xml version="1.0" encoding="UTF-8"?>
<container>
  <id type="integer">1</id>
  <sti-outer-objs type="array">
    <sti-outer-obj type="StiOuterDerivedObj">
      <id type="integer">1</id>
      <container-id type="integer">1</container-id>
      <sti-inner-obj>
        <id type="integer">1</id>
        <sti-outer-obj-id type="integer">1</sti-outer-obj-id>
      </sti-inner-obj>
    </sti-outer-obj>
  </sti-outer-objs>
</container>

Note that the inner object is incorrectly claimed to be of type sti_inner_obj when in fact it is sti_inner_derived_obj. That is, its true type is not being determined by the serializer code and instead, just by the association. This makes it impossible to unserialize correctly.

@cliochris

The following patch resolves the problem in Rails 3.2.8:

# See https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/6406-activerecordxmlserializer-fails-with-has_one-types
# See https://github.com/rails/rails/issues/7471 for the issue logged against the Rails 3.2.8 branch.
module ActiveModel
  module Serializers
    module Xml
      class Serializer
        private
          def add_associations(association, records, opts)
            merged_options = opts.merge(options.slice(:builder, :indent))
            merged_options[:skip_instruct] = true

            if records.is_a?(Enumerable)
              tag  = ActiveSupport::XmlMini.rename_key(association.to_s, options)
              type = options[:skip_types] ? { } : {:type => "array"}
              association_name = association.to_s.singularize
              merged_options[:root] = association_name

              if records.empty?
                @builder.tag!(tag, type)
              else
                @builder.tag!(tag, type) do
                  records.each do |record|
                    if options[:skip_types]
                      record_type = {}
                    else
                      record_class = (record.class.to_s.underscore == association_name) ? nil : record.class.name
                      record_type = {:type => record_class}
                    end

                    record.to_xml merged_options.merge(record_type)
                  end
                end
              end
            else
              merged_options[:root] = association.to_s
              association_name = association.to_s.singularize
              record_type = { :type => ((records.class.to_s.underscore == association_name) ? nil : records.class.name) }
              records.to_xml merged_options.merge(record_type)
            end
          end
      end
    end
  end
end
@cliochris

@acapilleri I have to use type as the whole point is that the failure happens when using single-table inheritance (STI). I agree that you don't want to use a column called type UNLESS you are using STI. Note that renaming it AND updating the has_one relationship (if possible) to use the new column name would not solve the problem. The problem is that has_one relationships ignore STI when being serialised to xml.

@acapilleri

@cliochris I have reproduced that, can u make a Pull Request if you have time?

@acapilleri

..and why don't u use ?

  to_xml({:include=>{:sti_outer_objs=>{:include=>:sti_inner_obj}}})
@cliochris

@acapilleri to your question about my use of to_xml, there'a an answer but I assure you it is not relevant to this discussion. :)

I'll try to do up a pull request over the next few days. Should be easy enough. Thanks for taking a look.

@cliochris

@acapilleri and anyone else interested, I have created a pull request:
#7849

@cliochris cliochris added a commit to cliochris/rails that referenced this issue Oct 5, 2012
@cliochris cliochris When serialising a class, specify the type of any singular associatio…
…ns, if

necessary. Rails already correctly specifies the :type of any enumerable
association (e.g. a has_many association), but made no attempt to do so for
non-enumerables (e.g. a has_one association).
We must specify the :type of any polymorphic association. A has_one
association to a class which uses single-table inheritance is an example of
a polymorphic association.
Fixes #7471
e7f7e35
@cliochris

@acapilleri, new pull rquest at #7858

I added a test case which demonstrates the problem in 3-2-stable and which passes with my pull request. I added a more descriptive commit message. Please let me know if there's anything more you would like from me.

@steveklabnik steveklabnik added a commit that closed this issue Nov 28, 2012
@steveklabnik steveklabnik Specify type of singular association during serialization
When serialising a class, specify the type of any singular associations, if
necessary. Rails already correctly specifies the :type of any enumerable
association (e.g. a has_many association), but made no attempt to do so for
non-enumerables (e.g. a has_one association).
We must specify the :type of any STI association. A has_one
association to a class which uses single-table inheritance is an example of
this type of association.

Fixes #7471
9504b44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.