Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gyoku.xml() method is not idempotent #40

Open
mcclumpherty opened this issue May 12, 2014 · 3 comments
Open

Gyoku.xml() method is not idempotent #40

mcclumpherty opened this issue May 12, 2014 · 3 comments

Comments

@mcclumpherty
Copy link

The xml method does not behave as I would expect as it modifies the hash that is passed into it. Moreover, it modifies elements within the hash as it proceeds. Consider the following code:

nil_content = { :content! => nil, :'@xs1:nil' => 1 }
bar = { request: { "attributes" => { "text" => nil_content, "date" => nil_content } }}
bar_xml = Gyoku.xml(bar)
puts bar_xml

This produces:

Note that the xs1:nil attribute is not set in the XML when it should be. This is because somehow Gyoku is modifying the content of the nil_content variable.

By changing the bar_xml assignment to use the Rails deep_dup() Hash method to:

bar_xml = Gyoku.xml(bar.deep_dup)
puts bar_xml

This produces:

I note that in the implementation it performs a dup on the hash that is called, but that is insufficient. It should perform (the equivalent) a deep_dup instead.

@tjarratt
Copy link
Contributor

Thanks for catching this @mcclumpherty. I can only wonder how many people were bit by this bug before and never spoke up about it. Is this something you'd be interested in submitting a pull request for?

@mcclumpherty
Copy link
Author

On reflection, I wonder if a deep_dup is a bit of a cheap and nasty solution. Not having looked through the implementation in detail, I wonder why the hash is being modified in the first place. Maybe the right solution would be not to do that. I'll have a think about submitting a pull request when I get the time.

@tjarratt
Copy link
Contributor

I suspect the cause is that ruby objects are always passed by reference and the call to #dup doesn't recurse into the hash's keys and values. I'd consider using @balmma's ruby_deep_clone gem for this behavior, although there might be something better. ruby_deep_clone seems to be fairly performant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants