Skip to content

Commit

Permalink
to_xml should also rely on serializable hash.
Browse files Browse the repository at this point in the history
  • Loading branch information
josevalim committed Sep 18, 2011
1 parent cb0dbe3 commit 51bef9d
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 32 deletions.
46 changes: 15 additions & 31 deletions activemodel/lib/active_model/serializers/xml.rb
Expand Up @@ -15,10 +15,10 @@ class Serializer #:nodoc:
class Attribute #:nodoc:
attr_reader :name, :value, :type

def initialize(name, serializable, raw_value=nil)
def initialize(name, serializable, value)
@name, @serializable = name, serializable
raw_value = raw_value.in_time_zone if raw_value.respond_to?(:in_time_zone)
@value = raw_value || @serializable.send(name)
value = value.in_time_zone if value.respond_to?(:in_time_zone)
@value = value
@type = compute_type
end

Expand Down Expand Up @@ -49,40 +49,24 @@ class MethodAttribute < Attribute #:nodoc:
def initialize(serializable, options = nil)
@serializable = serializable
@options = options ? options.dup : {}

@options[:only] = Array.wrap(@options[:only]).map { |n| n.to_s }
@options[:except] = Array.wrap(@options[:except]).map { |n| n.to_s }
end

# To replicate the behavior in ActiveRecord#attributes, <tt>:except</tt>
# takes precedence over <tt>:only</tt>. If <tt>:only</tt> is not set
# for a N level model but is set for the N+1 level models,
# then because <tt>:except</tt> is set to a default value, the second
# level model can have both <tt>:except</tt> and <tt>:only</tt> set. So if
# <tt>:only</tt> is set, always delete <tt>:except</tt>.
def attributes_hash
attributes = @serializable.attributes
if options[:only].any?
attributes.slice(*options[:only])
elsif options[:except].any?
attributes.except(*options[:except])
else
attributes
end
def serializable_hash
@serializable.serializable_hash(@options.except(:include))
end

def serializable_attributes
attributes_hash.map do |name, value|
self.class::Attribute.new(name, @serializable, value)
def serializable_collection
methods = Array.wrap(options[:methods]).map(&:to_s)
serializable_hash.map do |name, value|
name = name.to_s
if methods.include?(name)
self.class::MethodAttribute.new(name, @serializable, value)
else
self.class::Attribute.new(name, @serializable, value)
end
end
end

def serializable_methods
Array.wrap(options[:methods]).map do |name|
self.class::MethodAttribute.new(name.to_s, @serializable) if @serializable.respond_to?(name.to_s)
end.compact
end

def serialize
require 'builder' unless defined? ::Builder

Expand Down Expand Up @@ -114,7 +98,7 @@ def add_extra_behavior
end

def add_attributes_and_methods
(serializable_attributes + serializable_methods).each do |attribute|
serializable_collection.each do |attribute|
key = ActiveSupport::XmlMini.rename_key(attribute.name, options)
ActiveSupport::XmlMini.to_tag(key, attribute.value,
options.merge(attribute.decorations))
Expand Down
17 changes: 17 additions & 0 deletions activemodel/test/cases/serializers/xml_serialization_test.rb
Expand Up @@ -33,6 +33,12 @@ def attributes
end
end

class SerializableContact < Contact
def serializable_hash(options={})
super(options.merge(:only => [:name, :age]))
end
end

class XmlSerializationTest < ActiveModel::TestCase
def setup
@contact = Contact.new
Expand Down Expand Up @@ -96,6 +102,17 @@ def setup
assert_match %r{<createdAt}, @xml
end

test "should use serialiable hash" do
@contact = SerializableContact.new
@contact.name = 'aaron stack'
@contact.age = 25

@xml = @contact.to_xml
assert_match %r{<name>aaron stack</name>}, @xml
assert_match %r{<age type="integer">25</age>}, @xml
assert_no_match %r{<awesome>}, @xml
end

test "should allow skipped types" do
@xml = @contact.to_xml :skip_types => true
assert_match %r{<age>25</age>}, @xml
Expand Down
Expand Up @@ -179,7 +179,7 @@ def to_xml(options = {}, &block)
class XmlSerializer < ActiveModel::Serializers::Xml::Serializer #:nodoc:
def initialize(*args)
super
options[:except] |= Array.wrap(@serializable.class.inheritance_column)
options[:except] = Array.wrap(options[:except]) | Array.wrap(@serializable.class.inheritance_column)
end

class Attribute < ActiveModel::Serializers::Xml::Serializer::Attribute #:nodoc:
Expand Down

4 comments on commit 51bef9d

@deepakprasanna
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

This commit has broken a test.

  1. Failure:
    test_to_xml_with_private_method_name_as_attribute(BaseTest) [/home/shikeb/rails/activeresource/test/cases/base_test.rb:1007]:
    Exception raised:
    ArgumentError: wrong number of arguments (0 for 2..3).

289 tests, 892 assertions, 1 failures, 0 errors, 0 skips

Test run options: --seed 46552

Thanks.

cc @josevalim @spastorino

@jonleighton
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@josevalim I have fixed this in a15424b, please could you check that you are happy with the fix?

@josevalim
Copy link
Contributor Author

@josevalim josevalim commented on 51bef9d Sep 26, 2011 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonleighton
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree with you. I also think that our reliance on method_missing is a bit problematic, for example consider this:

def foo
  "bar"
end
class Person < ActiveResource::Base; end
Person.new(:foo => "baz").foo # => "bar"

It's not straightforward to avoid though. For example, in Active Record you can "piggyback" attributes onto a model: Person.select("people.*, 'foo' as bar") - this either requires using method_missing, or adding instance-specific behaviour, which is very slow as it requires creating a singleton class.

Please sign in to comment.