Make serializable_hash take attr values directly from attributes hash.

Previously, it would use send() to get the attribute. In Active
Resource, this would rely on hitting method missing. If a method with
the same name was defined further up the ancestor chain, that method
would wrongly be called.

This change fixes test_to_xml_with_private_method_name_as_attribute in
activeresource/test/cases/base_test.rb, which was broken after
51bef9d8fb0b4da7a104425ab8545e9331387743, because that change made
to_xml use serializable_hash.

3 files changed, 23 insertions, 4 deletions

diff --git a/activemodel/lib/active_model/serialization.rb b/activemodel/lib/active_model/serialization.rb
index b9f6f6cbbf..a756b9f205 100644
--- a/activemodel/lib/active_model/serialization.rb
+++ b/activemodel/lib/active_model/serialization.rb
@@ -78,8 +78,10 @@ module ActiveModel
         attribute_names -= Array.wrap(except).map(&:to_s)
       end
 
+      hash = attributes.slice(*attribute_names)
+
       method_names = Array.wrap(options[:methods]).select { |n| respond_to?(n) }
-      hash = Hash[(attribute_names + method_names).map { |n| [n, send(n)] }]
+      method_names.each { |n| hash[n] = send(n) }
 
       serializable_add_includes(options) do |association, records, opts|
         hash[association] = if records.is_a?(Enumerable)
diff --git a/activemodel/test/cases/serialization_test.rb b/activemodel/test/cases/serialization_test.rb
index 071cb9ff4e..29bcdeae67 100644
--- a/activemodel/test/cases/serialization_test.rb
+++ b/activemodel/test/cases/serialization_test.rb
@@ -77,6 +77,15 @@ class SerializationTest < ActiveModel::TestCase
     assert_equal expected , @user.serializable_hash(:methods => [:bar])
   end
 
+  def test_should_not_call_methods_for_attributes
+    def @user.name
+      "Jon"
+    end
+
+    expected = { "name" => "David" }
+    assert_equal expected, @user.serializable_hash(:only => :name)
+  end
+
   def test_include_option_with_singular_association
     expected =  {"name"=>"David", "gender"=>"male", "email"=>"david@example.com",
                  :address=>{"street"=>"123 Lane", "city"=>"Springfield", "state"=>"CA", "zip"=>11111}}
diff --git a/activeresource/test/cases/base_test.rb b/activeresource/test/cases/base_test.rb
index d4063fa299..7b42f64a35 100644
--- a/activeresource/test/cases/base_test.rb
+++ b/activeresource/test/cases/base_test.rb
@@ -1004,9 +1004,17 @@ class BaseTest < Test::Unit::TestCase
 
   def test_to_xml_with_private_method_name_as_attribute
     Person.format = :xml
-    assert_nothing_raised(ArgumentError) {
-      Customer.new(:test => true).to_xml
-    }
+
+    customer = Customer.new(:foo => "foo")
+    customer.singleton_class.class_eval do
+      def foo
+        "bar"
+      end
+      private :foo
+    end
+
+    assert !customer.to_xml.include?("<foo>bar</foo>")
+    assert customer.to_xml.include?("<foo>foo</foo>")
   ensure
     Person.format = :json
   end