diff options
| -rw-r--r-- | activeresource/lib/active_resource/connection.rb | 25 | ||||
| -rw-r--r-- | activeresource/test/base/custom_methods_test.rb | 5 | ||||
| -rw-r--r-- | activeresource/test/base_test.rb | 16 | ||||
| -rw-r--r-- | activeresource/test/connection_test.rb | 2 | ||||
| -rw-r--r-- | activesupport/CHANGELOG | 22 | ||||
| -rw-r--r-- | activesupport/lib/active_support/core_ext/array/conversions.rb | 13 | ||||
| -rw-r--r-- | activesupport/lib/active_support/core_ext/hash/conversions.rb | 14 | ||||
| -rw-r--r-- | activesupport/test/core_ext/array_ext_test.rb | 4 | ||||
| -rw-r--r-- | activesupport/test/core_ext/hash_ext_test.rb | 43 |
9 files changed, 106 insertions, 38 deletions
diff --git a/activeresource/lib/active_resource/connection.rb b/activeresource/lib/active_resource/connection.rb index d41171270c..8245c797b3 100644 --- a/activeresource/lib/active_resource/connection.rb +++ b/activeresource/lib/active_resource/connection.rb @@ -81,11 +81,7 @@ module ActiveResource 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 @@ module ActiveResource # 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 diff --git a/activeresource/test/base/custom_methods_test.rb b/activeresource/test/base/custom_methods_test.rb index f591c14260..0e1c00c478 100644 --- a/activeresource/test/base/custom_methods_test.rb +++ b/activeresource/test/base/custom_methods_test.rb @@ -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 @@ class CustomMethodsTest < Test::Unit::TestCase 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 diff --git a/activeresource/test/base_test.rb b/activeresource/test/base_test.rb index 6c924447de..328d3ae133 100644 --- a/activeresource/test/base_test.rb +++ b/activeresource/test/base_test.rb @@ -9,6 +9,10 @@ class BaseTest < Test::Unit::TestCase @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 @@ class BaseTest < Test::Unit::TestCase 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,7 +229,7 @@ class BaseTest < Test::Unit::TestCase 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 @@ -233,7 +237,7 @@ class BaseTest < Test::Unit::TestCase 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 @@ -241,7 +245,7 @@ class BaseTest < Test::Unit::TestCase 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 diff --git a/activeresource/test/connection_test.rb b/activeresource/test/connection_test.rb index 13b518d782..24893f8cd3 100644 --- a/activeresource/test/connection_test.rb +++ b/activeresource/test/connection_test.rb @@ -115,7 +115,7 @@ class ConnectionTest < Test::Unit::TestCase def test_get_collection_empty people = @conn.get("/people_empty_elements.xml") - assert_nil people + assert_equal [], people end def test_post diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG index 208b276624..75ff34dce9 100644 --- a/activesupport/CHANGELOG +++ b/activesupport/CHANGELOG @@ -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] diff --git a/activesupport/lib/active_support/core_ext/array/conversions.rb b/activesupport/lib/active_support/core_ext/array/conversions.rb index a4d056e81c..426e53ea1c 100644 --- a/activesupport/lib/active_support/core_ext/array/conversions.rb +++ b/activesupport/lib/active_support/core_ext/array/conversions.rb @@ -63,10 +63,15 @@ module ActiveSupport #:nodoc: 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 diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb index 2334bb671b..2cccd9c30e 100644 --- a/activesupport/lib/active_support/core_ext/hash/conversions.rb +++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb @@ -163,6 +163,20 @@ module ActiveSupport #:nodoc: 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 diff --git a/activesupport/test/core_ext/array_ext_test.rb b/activesupport/test/core_ext/array_ext_test.rb index e5ca4c424f..8c5356ac78 100644 --- a/activesupport/test/core_ext/array_ext_test.rb +++ b/activesupport/test/core_ext/array_ext_test.rb @@ -121,7 +121,7 @@ class ArrayToXmlTests < Test::Unit::TestCase { :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 @@ class ArrayToXmlTests < Test::Unit::TestCase { :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 diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index a428fe5061..cae19a9335 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -370,7 +370,7 @@ class HashToXmlTest < Test::Unit::TestCase 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>)) @@ -378,7 +378,7 @@ class HashToXmlTest < Test::Unit::TestCase 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 @@ class HashToXmlTest < Test::Unit::TestCase 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 @@ class HashToXmlTest < Test::Unit::TestCase :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 @@ class HashToXmlTest < Test::Unit::TestCase 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 |