[AS] XML behavior processing

[leandronsp]

Scenario

If I have a xml like this:

<varValue name="age">25</varValue>

Running the 'from_xml' we get this:

Hash.from_xml(xml)
=> {"varValue"=>"25"}

# Losing my 'name' attribute :(

Code

.activesupport/lib/active_support/core_ext/hash/conversions.rb

When the hash is being processed, this:

{"name"=>"age", "__content__"=>"25"}

...is eligible for 'become_content'. Then ASupport ignores the 'name' attribute.
I'd like to get something _like_ this:

{"varValue"=>{"name"=>"age", "__content__"=>"25"}}

Initial solution

I realized a solution for that by changing the OR clause for a AND clause.
From:

def become_content?(value)                                                                                                        
value['type'] == 'file' || (value['__content__'] && (value.keys.size == 1 || value['__content__'].present?))                    
end

To:

def become_content?(value)                                                                                                        
value['type'] == 'file' || (value['__content__'] && (value.keys.size == 1 && value['__content__'].present?))                    
end

Summarize:

value.keys.size == 1 && value['__content__'].present?

But I don't know if this is the proper solution.
What about it?

[leandronsp]

Trying to be clear, I mean that the hash:

{"name"=>"age", "__content__"=>"25"}

should not be eligible as 'become_content'.
Then the proposed expression:

value.keys.size == 1 && value['__content__'].present?

would return _false_, and the hash would be eligible as 'become_hash', giving me the complete hash I want:

{"varValue"=>{"name"=>"age", "__content__"=>"25"}}

[vipulnsward]

makes sense.
So https://github.com/rails/rails/blob/master/activesupport/lib/active_support/core_ext/hash/conversions.rb#L196 will be changed.

Maybe it could change to

value['type'] == 'file' || (value['__content__'].present? && value.keys.size == 1 )

[leandronsp]

Running the test suite, I got some failures.

The method become_content? could be changed to:

def become_content?(value)                                                                                                        
  return true if value['type'] == 'file'                                                                                          
  return false unless value.has_key?('__content__')                                                                               

  value.keys.size == 1 || value['type'].present?                                                                                  
end

However, according the convention, if some XML legacy brings a type attribute (which would not be a type in fact), this solution won't work, skipping the type

[vipulnsward]

oddly doing something like

def become_content?(value)
  value['type'] == 'file' || [["type", "encoding", "__content__"] , ["type", "__content__"] , ["__content__"]].include?(value.keys)
end

works

@rafaelfranca @carlosantoniodasilva thoughts on if above could be a good thing to do? It satisfies all previous conditions and the one that is reported, as also it is easier to understand.

If yes, I would go ahead with a PR.

[nickl-]

Also from unaddressed legacy ticket #588
PR #9985

[rafaelfranca]

This issue has been automatically marked as stale because it has not been commented on for at least
three months.

The resources of the Rails team are limited, and so we are asking for your help.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

[rails-bot]

This issue has been automatically closed because of inactivity.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.