From e01dfb27fc5573db9070dce788f2e5ee62094722 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Fri, 15 Apr 2011 13:09:12 +0100 Subject: Undo performances regressions I introduced in bbe0a507f287c20ab4ae8a244fbfc810665deda5 and add test for an edge case. Add comments to explain the intent of the code. --- .../lib/active_record/attribute_methods/read.rb | 28 ++++++++++++++++++---- activerecord/test/cases/attribute_methods_test.rb | 11 +++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index dd3f4699f7..cd6c4002d2 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -43,7 +43,7 @@ module ActiveRecord end if attr_name == primary_key && attr_name != "id" - define_read_method(:id, attr_name, columns_hash[attr_name]) + define_read_method('id', attr_name, columns_hash[attr_name]) end end @@ -59,7 +59,9 @@ module ActiveRecord end # Define an attribute reader method. Cope with nil column. - def define_read_method(symbol, attr_name, column) + # method_name is the same as attr_name except when a non-standard primary key is used, + # we still define #id as an accessor for the key + def define_read_method(method_name, attr_name, column) cast_code = column.type_cast_code('v') access_code = "(v=@attributes['#{attr_name}']) && #{cast_code}" @@ -71,9 +73,25 @@ module ActiveRecord access_code = "@attributes_cache['#{attr_name}'] ||= (#{access_code})" end - generated_attribute_methods.module_eval do - define_method("_#{attr_name}") { eval(access_code) } - alias_method(attr_name, "_#{attr_name}") + # Where possible, generate the method by evalling a string, as this will result in + # faster accesses because it avoids the block eval and then string eval incurred + # by the second branch. + # + # The second, slower, branch is necessary to support instances where the database + # returns columns with extra stuff in (like 'my_column(omg)'). + if method_name =~ /^[a-zA-Z_]\w*[!?=]?$/ + generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + def _#{method_name} + #{access_code} + end + + alias #{method_name} _#{method_name} + STR + else + generated_attribute_methods.module_eval do + define_method("_#{method_name}") { eval(access_code) } + alias_method(method_name, "_#{method_name}") + end end end end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 899166d129..5074ae50ab 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -10,6 +10,7 @@ require 'models/company' require 'models/category' require 'models/reply' require 'models/contact' +require 'models/keyboard' class AttributeMethodsTest < ActiveRecord::TestCase fixtures :topics, :developers, :companies, :computers @@ -89,6 +90,16 @@ class AttributeMethodsTest < ActiveRecord::TestCase assert !topic.respond_to?(:nothingness) end + def test_respond_to_with_custom_primary_key + keyboard = Keyboard.create + assert_not_nil keyboard.key_number + assert_equal keyboard.key_number, keyboard.id + assert keyboard.respond_to?('key_number') + assert keyboard.respond_to?('_key_number') + assert keyboard.respond_to?('id') + assert keyboard.respond_to?('_id') + end + # Syck calls respond_to? before actually calling initialize def test_respond_to_with_allocated_object topic = Topic.allocate -- cgit v1.2.3