Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Merge pull request #8471 from kytrinyx/refactor-xml-to-hash

WIP Refactor xml conversion to hash
  • Loading branch information...
commit 10c0a3bd113c41f44fc025d2d042da95e9d8ea1f 2 parents bc8d2f6 + b02ebe7
@steveklabnik steveklabnik authored
View
150 activesupport/lib/active_support/core_ext/hash/conversions.rb
@@ -102,55 +102,41 @@ class << self
# hash = Hash.from_xml(xml)
# # => {"hash"=>{"foo"=>1, "bar"=>2}}
def from_xml(xml)
- typecast_xml_value(unrename_keys(ActiveSupport::XmlMini.parse(xml)))
+ ActiveSupport::XMLConverter.new(xml).to_h
+ end
+
+ end
+end
+
+module ActiveSupport
+ class XMLConverter # :nodoc:
+ def initialize(xml)
+ @xml = normalize_keys(XmlMini.parse(xml))
+ end
+
+ def to_h
+ deep_to_h(@xml)
end
private
- def typecast_xml_value(value)
- case value
+
+ def normalize_keys(params)
+ case params
when Hash
- if value['type'] == 'array'
- _, entries = Array.wrap(value.detect { |k,v| not v.is_a?(String) })
- if entries.nil? || (c = value['__content__'] && c.blank?)
- []
- else
- case entries # 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'] == 'file' ||
- (value['__content__'] && (value.keys.size == 1 || value['__content__'].present?))
- content = value['__content__']
- if parser = ActiveSupport::XmlMini::PARSING[value['type']]
- parser.arity == 1 ? parser.call(content) : parser.call(content, value)
- else
- content
- end
- elsif value['type'] == 'string' && value['nil'] != 'true'
- ''
- # blank or nil parsed values are represented by nil
- elsif value.blank? || value['nil'] == 'true'
- nil
- # If the type is the only element which makes it then
- # this still makes the value nil, except if type is
- # a XML node(where type['value'] is a Hash)
- elsif value['type'] && value.size == 1 && !value['type'].is_a?(::Hash)
- nil
- else
- xml_value = Hash[value.map { |k,v| [k, typecast_xml_value(v)] }]
+ Hash[params.map { |k,v| [k.to_s.tr('-', '_'), normalize_keys(v)] } ]
+ when Array
+ params.map { |v| normalize_keys(v) }
+ else
+ params
+ end
+ end
- # Turn { files: { file: #<StringIO> } } into { files: #<StringIO> } so it is compatible with
- # how multipart uploaded files from HTML appear
- xml_value['file'].is_a?(StringIO) ? xml_value['file'] : xml_value
- end
+ def deep_to_h(value)
+ case value
+ when Hash
+ process_hash(value)
when Array
- value.map! { |i| typecast_xml_value(i) }
- value.length > 1 ? value : value.first
+ process_array(value)
when String
value
else
@@ -158,15 +144,79 @@ def typecast_xml_value(value)
end
end
- def unrename_keys(params)
- case params
- when Hash
- Hash[params.map { |k,v| [k.to_s.tr('-', '_'), unrename_keys(v)] } ]
- when Array
- params.map { |v| unrename_keys(v) }
+ def process_hash(value)
+ if become_array?(value)
+ _, entries = Array.wrap(value.detect { |k,v| not v.is_a?(String) })
+ if entries.nil? || value['__content__'].try(:empty?)
+ []
else
- params
+ case entries
+ when Array
+ entries.collect { |v| deep_to_h(v) }
+ when Hash
+ [deep_to_h(entries)]
+ else
+ raise "can't typecast #{entries.inspect}"
+ end
+ end
+ elsif become_content?(value)
+ process_content(value)
+
+ elsif become_empty_string?(value)
+ ''
+ elsif become_hash?(value)
+ xml_value = Hash[value.map { |k,v| [k, deep_to_h(v)] }]
+
+ # Turn { files: { file: #<StringIO> } } into { files: #<StringIO> } so it is compatible with
+ # how multipart uploaded files from HTML appear
+ xml_value['file'].is_a?(StringIO) ? xml_value['file'] : xml_value
end
end
+
+ def become_content?(value)
+ value['type'] == 'file' || (value['__content__'] && (value.keys.size == 1 || value['__content__'].present?))
+ end
+
+ def become_array?(value)
+ value['type'] == 'array'
+ end
+
+ def become_empty_string?(value)
+ # {"string" => true}
+ # No tests fail when the second term is removed.
+ value['type'] == 'string' && value['nil'] != 'true'
+ end
+
+ def become_hash?(value)
+ !nothing?(value) && !garbage?(value)
+ end
+
+ def nothing?(value)
+ # blank or nil parsed values are represented by nil
+ value.blank? || value['nil'] == 'true'
+ end
+
+ def garbage?(value)
+ # If the type is the only element which makes it then
+ # this still makes the value nil, except if type is
+ # a XML node(where type['value'] is a Hash)
+ value['type'] && !value['type'].is_a?(::Hash) && value.size == 1
+ end
+
+ def process_content(value)
+ content = value['__content__']
+ if parser = ActiveSupport::XmlMini::PARSING[value['type']]
+ parser.arity == 1 ? parser.call(content) : parser.call(content, value)
+ else
+ content
+ end
+ end
+
+ def process_array(value)
+ value.map! { |i| deep_to_h(i) }
+ value.length > 1 ? value : value.first
+ end
+
end
end
+
View
2  activesupport/test/core_ext/hash_ext_test.rb
@@ -1330,7 +1330,7 @@ def test_kernel_method_names_to_xml
def test_empty_string_works_for_typecast_xml_value
assert_nothing_raised do
- Hash.__send__(:typecast_xml_value, "")
+ ActiveSupport::XMLConverter.new("").to_h
end
end
Please sign in to comment.
Something went wrong with that request. Please try again.