Skip to content

Loading…

Use `tag!` instead of `method_missing` in `to_xml` conversions. #8111

Merged
merged 1 commit into from

2 participants

@nikitug

Since version 3.0.x Builder caches method passed to method_missing each time. This commit replaces method_missing call with tag! call to prevent method redefinition on each to_xml call with the same builder.

@carlosantoniodasilva
Ruby on Rails member

Pull request seems fine. But I imagine that if it has caching now, when you call to_xml a second time with the same builder, it'd hit the defined method (the "cached" one) instead of method_missing, which is expected, or not? Have you found any issue with this caching? Thanks.

@nikitug

@carlosantoniodasilva not exactly, there was an explicit method_missing call, like builder.send(:method_missing, tag). So it got called on each hash inside array, for example, producing tons of warnings.

@carlosantoniodasilva
Ruby on Rails member

@nikitug true, it was probably to avoid all method lookup before hitting method missing =(. Mind adding a changelog entry for this change? Thanks.

@nikitug nikitug Use `tag!` instead of `method_missing` in `to_xml` conversions.
Since version `3.0.x` `Builder` caches method passed to `method_missing` each time. This commit replaces `method_missing` call with `tag!` call to prevent method redefinition on each `to_xml` call with the same builder.
9cda6a3
@nikitug

@carlosantoniodasilva yes, and that worked fine with old Builder's method_missing/tag implementation, but was broken with the new version. Sure, added a changelog entry. Thanks.

@carlosantoniodasilva
Ruby on Rails member

Great, thanks!

@carlosantoniodasilva carlosantoniodasilva merged commit f786469 into rails:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 4, 2012
  1. @nikitug

    Use `tag!` instead of `method_missing` in `to_xml` conversions.

    nikitug committed
    Since version `3.0.x` `Builder` caches method passed to `method_missing` each time. This commit replaces `method_missing` call with `tag!` call to prevent method redefinition on each `to_xml` call with the same builder.
View
4 activesupport/CHANGELOG.md
@@ -1,5 +1,9 @@
## Rails 4.0.0 (unreleased) ##
+* `to_xml` conversions now use builder's `tag!` method instead of explicit invocation of `method_missing`.
+
+ *Nikita Afanasenko*
+
* Fixed timezone mapping of the Solomon Islands. *Steve Klabnik*
* Make callstack attribute optional in
View
6 activesupport/lib/active_support/core_ext/array/conversions.rb
@@ -194,7 +194,7 @@ def to_xml(options = {})
options = options.dup
options[:indent] ||= 2
- options[:builder] ||= Builder::XmlMarkup.new(:indent => options[:indent])
+ options[:builder] ||= Builder::XmlMarkup.new(indent: options[:indent])
options[:root] ||= \
if first.class != Hash && all? { |e| e.is_a?(first.class) }
underscored = ActiveSupport::Inflector.underscore(first.class.name)
@@ -208,12 +208,12 @@ def to_xml(options = {})
root = ActiveSupport::XmlMini.rename_key(options[:root].to_s, options)
children = options.delete(:children) || root.singularize
- attributes = options[:skip_types] ? {} : {:type => 'array'}
+ attributes = options[:skip_types] ? {} : { type: 'array' }
if empty?
builder.tag!(root, attributes)
else
- builder.__send__(:method_missing, root, attributes) do
+ builder.tag!(root, attributes) do
each { |value| ActiveSupport::XmlMini.to_tag(children, value, options) }
yield builder if block_given?
end
View
4 activesupport/lib/active_support/core_ext/hash/conversions.rb
@@ -74,14 +74,14 @@ def to_xml(options = {})
options = options.dup
options[:indent] ||= 2
options[:root] ||= 'hash'
- options[:builder] ||= Builder::XmlMarkup.new(:indent => options[:indent])
+ options[:builder] ||= Builder::XmlMarkup.new(indent: options[:indent])
builder = options[:builder]
builder.instruct! unless options.delete(:skip_instruct)
root = ActiveSupport::XmlMini.rename_key(options[:root].to_s, options)
- builder.__send__(:method_missing, root) do
+ builder.tag!(root) do
each { |key, value| ActiveSupport::XmlMini.to_tag(key, value, options) }
yield builder if block_given?
end
Something went wrong with that request. Please try again.