Permalink
Browse files

Undo performances regressions I introduced in 36691ac and add test fo…

…r an edge case. Add comments to explain the intent of the code. Also fix the code (which previously worked differently on 1.8 and 1.9 due to Symbol#=~ being always false on 1.8)."
  • Loading branch information...
1 parent 3609642 commit ef88c0c2bb1c119c27f2d0e9e11a6704c560a7c1 @jonleighton jonleighton committed Apr 15, 2011
@@ -40,11 +40,11 @@ def define_method_attribute(attr_name)
if self.serialized_attributes[attr_name]
define_read_method_for_serialized_attribute(attr_name)
else
- define_read_method(attr_name.to_sym, attr_name, columns_hash[attr_name])
+ define_read_method(attr_name, attr_name, columns_hash[attr_name])
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
@@ -55,7 +55,9 @@ def define_read_method_for_serialized_attribute(attr_name)
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') if column
access_code = cast_code ? "(v=@attributes['#{attr_name}']) && #{cast_code}" : "@attributes['#{attr_name}']"
@@ -67,9 +69,25 @@ def define_read_method(symbol, attr_name, column)
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
@@ -8,6 +8,7 @@
require 'models/company'
require 'models/category'
require 'models/reply'
+require 'models/keyboard'
class AttributeMethodsTest < ActiveRecord::TestCase
fixtures :topics, :developers, :companies, :computers
@@ -87,6 +88,16 @@ def test_respond_to?
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

0 comments on commit ef88c0c

Please sign in to comment.