Skip to content

Commit

Permalink
Fixed Hash#from_xml with keys that are all caps.
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Koziarski <michael@koziarski.com>
  • Loading branch information
codebrulee authored and NZKoz committed May 4, 2009
1 parent 8b6d2ef commit eb201e6
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 1 deletion.
Expand Up @@ -217,7 +217,7 @@ def unrename_keys(params)
case params.class.to_s
when "Hash"
params.inject({}) do |h,(k,v)|
h[k.to_s.underscore.tr("-", "_")] = unrename_keys(v)
h[k.to_s.tr("-", "_")] = unrename_keys(v)
h
end
when "Array"
Expand Down
16 changes: 16 additions & 0 deletions activesupport/test/core_ext/hash_ext_test.rb
Expand Up @@ -646,6 +646,22 @@ 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_all_caps_key_from_xml
test_xml = <<-EOT
<ABC3XYZ>
<TEST>Lorem Ipsum</TEST>
</ABC3XYZ>
EOT

expected_hash = {
"ABC3XYZ" => {
"TEST" => "Lorem Ipsum"
}
}

assert_equal expected_hash, Hash.from_xml(test_xml)
end

def test_empty_array_from_xml
blog_xml = <<-XML
<blog>
Expand Down

3 comments on commit eb201e6

@whydealme
Copy link

Choose a reason for hiding this comment

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

This change is really breaking ActiveResource xml support. All ActiveResource XML based applications are broken because of this change.
For example:
Due to this 'MyElement' xml element wouldn't be converted to my_element object.. It would remain as MyElement.
I wonder if there were no tests to validate this condition.. If there were then they all would be red..

@dstrelau
Copy link

Choose a reason for hiding this comment

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

@whydealme: This actually fixes the fact that those elements should not be underscorized in the first place. See commit aa5cdb and Lighthouse #2604.

@dstrelau
Copy link

Choose a reason for hiding this comment

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

Let me try again: commit aa5cdb0 and Lighthouse 2604

Please sign in to comment.