Permalink
Browse files

Added proper handling of arrays. Closes #8537 [hasmanyjosh]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@7074 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
1 parent eb2e30e commit 9e4461438f8ce584b635aca35579c36537a340ca @technoweenie technoweenie committed Jun 21, 2007
@@ -81,11 +81,7 @@ def post(path, body = '', headers = {})
end
def xml_from_response(response)
- if response = from_xml_data(Hash.from_xml(response.body))
- response.first
- else
- nil
- end
+ from_xml_data(Hash.from_xml(response.body))
end
@@ -150,21 +146,12 @@ def logger #:nodoc:
# Manipulate from_xml Hash, because xml_simple is not exactly what we
# want for ActiveResource.
def from_xml_data(data)
- case data
- when Hash
- if data.keys.size == 1
- case data.values.first
- when Hash then [ from_xml_data(data.values.first) ]
- when Array then from_xml_data(data.values.first)
- else data.values.first
- end
- else
- data.each_key { |key| data[key] = from_xml_data(data[key]) }
- data
- end
- when Array then data.collect { |val| from_xml_data(val) }
- else data
+ if data.is_a?(Hash) && data.keys.size == 1
+ data.values.first
+ else
+ data
end
end
+
end
end
@@ -6,6 +6,7 @@ class CustomMethodsTest < Test::Unit::TestCase
def setup
@matz = { :id => 1, :name => 'Matz' }.to_xml(:root => 'person')
@matz_deep = { :id => 1, :name => 'Matz', :other => 'other' }.to_xml(:root => 'person')
+ @matz_array = [{ :id => 1, :name => 'Matz' }].to_xml(:root => 'people')
@ryan = { :name => 'Ryan' }.to_xml(:root => 'person')
@addy = { :id => 1, :street => '12345 Street' }.to_xml(:root => 'address')
@addy_deep = { :id => 1, :street => '12345 Street', :zip => "27519" }.to_xml(:root => 'address')
@@ -15,8 +16,8 @@ def setup
mock.get "/people/1.xml", {}, @matz
mock.get "/people/1/shallow.xml", {}, @matz
mock.get "/people/1/deep.xml", {}, @matz_deep
- mock.get "/people/retrieve.xml?name=Matz", {}, "<people>#{@matz}</people>"
- mock.get "/people/managers.xml", {}, "<people>#{@matz}</people>"
+ mock.get "/people/retrieve.xml?name=Matz", {}, @matz_array
+ mock.get "/people/managers.xml", {}, @matz_array
mock.put "/people/1/promote.xml?position=Manager", {}, nil, 204
mock.put "/people/promote.xml?name=Matz", {}, nil, 204, {}
mock.put "/people/sort.xml?by=name", {}, nil, 204
@@ -9,6 +9,10 @@ def setup
@david = { :id => 2, :name => 'David' }.to_xml(:root => 'person')
@addy = { :id => 1, :street => '12345 Street' }.to_xml(:root => 'address')
@default_request_headers = { 'Content-Type' => 'application/xml' }
+ @rick = { :name => "Rick", :age => 25 }.to_xml(:root => "person")
+ @people = [{ :id => 1, :name => 'Matz' }, { :id => 2, :name => 'David' }].to_xml(:root => 'people')
+ @people_david = [{ :id => 2, :name => 'David' }].to_xml(:root => 'people')
+ @addresses = [{ :id => 1, :street => '12345 Street' }].to_xml(:root => 'addresses')
ActiveResource::HttpMock.respond_to do |mock|
mock.get "/people/1.xml", {}, @matz
@@ -18,9 +22,9 @@ def setup
mock.delete "/people/1.xml", {}, nil, 200
mock.delete "/people/2.xml", {}, nil, 400
mock.get "/people/99.xml", {}, nil, 404
- mock.post "/people.xml", {}, "<person><name>Rick</name><age type='integer'>25</age></person>", 201, 'Location' => '/people/5.xml'
- mock.get "/people.xml", {}, "<people>#{@matz}#{@david}</people>"
- mock.get "/people/1/addresses.xml", {}, "<addresses>#{@addy}</addresses>"
+ mock.post "/people.xml", {}, @rick, 201, 'Location' => '/people/5.xml'
+ mock.get "/people.xml", {}, @people
+ mock.get "/people/1/addresses.xml", {}, @addresses
mock.get "/people/1/addresses/1.xml", {}, @addy
mock.get "/people/1/addresses/2.xml", {}, nil, 404
mock.get "/people/2/addresses/1.xml", {}, nil, 404
@@ -225,23 +229,23 @@ def test_find_by_id_not_found
end
def test_find_all_by_from
- ActiveResource::HttpMock.respond_to { |m| m.get "/companies/1/people.xml", {}, "<people>#{@david}</people>" }
+ ActiveResource::HttpMock.respond_to { |m| m.get "/companies/1/people.xml", {}, @people_david }
people = Person.find(:all, :from => "/companies/1/people.xml")
assert_equal 1, people.size
assert_equal "David", people.first.name
end
def test_find_all_by_from_with_options
- ActiveResource::HttpMock.respond_to { |m| m.get "/companies/1/people.xml", {}, "<people>#{@david}</people>" }
+ ActiveResource::HttpMock.respond_to { |m| m.get "/companies/1/people.xml", {}, @people_david }
people = Person.find(:all, :from => "/companies/1/people.xml")
assert_equal 1, people.size
assert_equal "David", people.first.name
end
def test_find_all_by_symbol_from
- ActiveResource::HttpMock.respond_to { |m| m.get "/people/managers.xml", {}, "<people>#{@david}</people>" }
+ ActiveResource::HttpMock.respond_to { |m| m.get "/people/managers.xml", {}, @people_david }
people = Person.find(:all, :from => :managers)
assert_equal 1, people.size
@@ -115,7 +115,7 @@ def test_get_collection_single
def test_get_collection_empty
people = @conn.get("/people_empty_elements.xml")
- assert_nil people
+ assert_equal [], people
end
def test_post
View
@@ -1,5 +1,27 @@
*SVN*
+* Added proper handling of arrays #8537 [hasmanyjosh]
+
+ Before:
+ Hash.from_xml '<images></images>'
+ # => {:images => nil}
+
+ Hash.from_xml '<images><image>foo.jpg</image></images>'
+ # => {:images => {:image => "foo.jpg"}}
+
+ Hash.from_xml '<images><image>foo.jpg</image><image>bar.jpg</image></images>'
+ # => {:images => {:image => ["foo.jpg", "bar.jpg"]}}
+
+ After:
+ Hash.from_xml '<images type="array"></images>'
+ # => {:images => []}
+
+ Hash.from_xml '<images type="array"><image>foo.jpg</image></images>'
+ # => {:images => ["foo.jpg"]}
+
+ Hash.from_xml '<images type="array"><image>foo.jpg</image><image>bar.jpg</image></images>'
+ # => {:images => ["foo.jpg", "bar.jpg"]}
+
* Improve Time and Date test coverage. #8646 [Josh Peek]
* Add Date#since, ago, beginning_of_day, and end_of_day. Date + seconds works now. #8575 [Geoff Buesing]
@@ -63,10 +63,15 @@ def to_xml(options = {})
opts = options.merge({ :root => children })
- options[:builder].tag!(root) {
- yield options[:builder] if block_given?
- each { |e| e.to_xml(opts.merge!({ :skip_instruct => true })) }
- }
+ xml = options[:builder]
+ if empty?
+ xml.tag!(root, options[:skip_types] ? {} : {:type => "array"})
+ else
+ xml.tag!(root, options[:skip_types] ? {} : {:type => "array"}) {
+ yield xml if block_given?
+ each { |e| e.to_xml(opts.merge!({ :skip_instruct => true })) }
+ }
+ end
end
end
@@ -163,6 +163,20 @@ def typecast_xml_value(value)
else
content
end
+ elsif value['type'] == 'array'
+ child_key, entries = value.detect { |k,v| k != 'type' } # child_key is throwaway
+ if entries.nil?
+ []
+ else
+ case entries.class.to_s # something weird with classes not matching here. maybe singleton methods breaking is_a?
+ when "Array"
+ entries.collect { |v| typecast_xml_value(v) }
+ when "Hash"
+ [typecast_xml_value(entries)]
+ else
+ raise "can't typecast #{entries.inspect}"
+ end
+ end
elsif value['type'] == 'string' && value['nil'] != 'true'
""
else
@@ -121,7 +121,7 @@ def test_to_xml
{ :name => "Jason", :age => 31, :age_in_millis => BigDecimal.new('1.0') }
].to_xml(:skip_instruct => true, :indent => 0)
- assert_equal "<records><record>", xml.first(17), xml
+ assert_equal '<records type="array"><record>', xml.first(30)
assert xml.include?(%(<age type="integer">26</age>)), xml
assert xml.include?(%(<age-in-millis type="integer">820497600000</age-in-millis>)), xml
assert xml.include?(%(<name>David</name>)), xml
@@ -135,7 +135,7 @@ def test_to_xml_with_dedicated_name
{ :name => "David", :age => 26, :age_in_millis => 820497600000 }, { :name => "Jason", :age => 31 }
].to_xml(:skip_instruct => true, :indent => 0, :root => "people")
- assert_equal "<people><person>", xml.first(16)
+ assert_equal '<people type="array"><person>', xml.first(29)
end
def test_to_xml_with_options
@@ -370,15 +370,15 @@ def test_two_levels_with_second_level_overriding_to_xml
def test_two_levels_with_array
xml = { :name => "David", :addresses => [{ :street => "Paulina" }, { :street => "Evergreen" }] }.to_xml(@xml_options)
assert_equal "<person>", xml.first(8)
- assert xml.include?(%(<addresses><address>))
+ assert xml.include?(%(<addresses type="array"><address>))
assert xml.include?(%(<address><street>Paulina</street></address>))
assert xml.include?(%(<address><street>Evergreen</street></address>))
assert xml.include?(%(<name>David</name>))
end
def test_three_levels_with_array
xml = { :name => "David", :addresses => [{ :streets => [ { :name => "Paulina" }, { :name => "Paulina" } ] } ] }.to_xml(@xml_options)
- assert xml.include?(%(<addresses><address><streets><street><name>))
+ assert xml.include?(%(<addresses type="array"><address><streets type="array"><street><name>))
end
def test_single_record_from_xml
@@ -447,7 +447,7 @@ def test_single_record_from_xml_with_nil_values
def test_multiple_records_from_xml
topics_xml = <<-EOT
- <topics>
+ <topics type="array">
<topic>
<title>The First Topic</title>
<author-name>David</author-name>
@@ -491,7 +491,7 @@ def test_multiple_records_from_xml
:parent_id => nil
}.stringify_keys
- assert_equal expected_topic_hash, Hash.from_xml(topics_xml)["topics"]["topic"].first
+ assert_equal expected_topic_hash, Hash.from_xml(topics_xml)["topics"].first
end
def test_single_record_from_xml_with_attributes_other_than_type
@@ -516,6 +516,41 @@ def test_single_record_from_xml_with_attributes_other_than_type
assert_equal expected_topic_hash, Hash.from_xml(topic_xml)["rsp"]["photos"]["photo"]
end
+
+ def test_empty_array_from_xml
+ blog_xml = <<-XML
+ <blog>
+ <posts type="array"></posts>
+ </blog>
+ XML
+ expected_blog_hash = {"blog" => {"posts" => []}}
+ assert_equal expected_blog_hash, Hash.from_xml(blog_xml)
+ end
+
+ def test_array_with_one_entry_from_xml
+ blog_xml = <<-XML
+ <blog>
+ <posts type="array">
+ <post>a post</post>
+ </posts>
+ </blog>
+ XML
+ expected_blog_hash = {"blog" => {"posts" => ["a post"]}}
+ assert_equal expected_blog_hash, Hash.from_xml(blog_xml)
+ end
+
+ def test_array_with_multiple_entries_from_xml
+ blog_xml = <<-XML
+ <blog>
+ <posts type="array">
+ <post>a post</post>
+ <post>another post</post>
+ </posts>
+ </blog>
+ XML
+ expected_blog_hash = {"blog" => {"posts" => ["a post", "another post"]}}
+ assert_equal expected_blog_hash, Hash.from_xml(blog_xml)
+ end
def test_xsd_like_types_from_xml
bacon_xml = <<-EOT

0 comments on commit 9e44614

Please sign in to comment.