Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Don't rely on underscore-prefixed attribute methods.

Define singleton methods on the attributes module instead. This reduces
method pollution on the actual model classes. It also seems to make
something faster, I am unsure why! O_o
  • Loading branch information...
commit 3dcb127109428c1948a9f3bcad7101bd8a7f4d8a 1 parent 365e10b
@jonleighton jonleighton authored
View
91 activerecord/lib/active_record/attribute_methods/read.rb
@@ -29,9 +29,54 @@ def cache_attribute?(attr_name)
cached_attributes.include?(attr_name)
end
+ def undefine_attribute_methods
+ if base_class == self
+ generated_attribute_methods.module_eval do
+ public_methods(false).each do |m|
+ singleton_class.send(:undef_method, m) if m.to_s =~ /^cast_/
+ end
+ end
+ end
+
+ super
+ end
+
protected
+ # 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)').
def define_method_attribute(attr_name)
- define_read_method(attr_name, attr_name, columns_hash[attr_name])
+ access_code = attribute_access_code(attr_name)
+ cast_code = "v && (#{attribute_cast_code(attr_name)})"
+
+ if attr_name =~ ActiveModel::AttributeMethods::NAME_COMPILABLE_REGEXP
+ generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__
+ def #{attr_name}
+ #{access_code}
+ end
+
+ def self.cast_#{attr_name}(v)
+ #{cast_code}
+ end
+
+ alias _#{attr_name} #{attr_name}
+ STR
+ else
+ generated_attribute_methods.module_eval do
+ define_method(attr_name) do
+ eval(access_code)
+ end
+
+ singleton_class.send(:define_method, "cast_#{attr_name}") do |v|
+ eval(cast_code)
+ end
+
+ alias_method("_#{attr_name}", attr_name)
+ end
+ end
end
private
@@ -39,14 +84,10 @@ def cacheable_column?(column)
attribute_types_cached_by_default.include?(column.type)
end
- # Define an attribute reader method. Cope with nil 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}"
+ def attribute_access_code(attr_name)
+ access_code = "(v=@attributes['#{attr_name}']) && #{attribute_cast_code(attr_name)}"
- unless attr_name.to_s == self.primary_key.to_s
+ unless attr_name == self.primary_key
access_code.insert(0, "missing_attribute('#{attr_name}', caller) unless @attributes.has_key?('#{attr_name}'); ")
end
@@ -54,35 +95,25 @@ def define_read_method(method_name, attr_name, column)
access_code = "@attributes_cache['#{attr_name}'] ||= (#{access_code})"
end
- # 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 =~ ActiveModel::AttributeMethods::NAME_COMPILABLE_REGEXP
- generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__
- def _#{method_name}
- #{access_code}
- end
+ 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
+ def attribute_cast_code(attr_name)
+ columns_hash[attr_name].type_cast_code('v')
end
end
# Returns the value of the attribute identified by <tt>attr_name</tt> after it has been typecast (for example,
# "2004-12-12" in a data column is cast to a date object, like Date.new(2004, 12, 12)).
def read_attribute(attr_name)
- method = "_#{attr_name}"
- if respond_to? method
- send method if @attributes.has_key?(attr_name.to_s)
+ attr_name = attr_name.to_s
+ caster = "cast_#{attr_name}"
+ methods = self.class.generated_attribute_methods
@Vanuan
Vanuan added a note

Single table inheritance is broken here.

@jonleighton Collaborator

I don't think so, because STI always has the same DB columns. However, the code is changed in later commits to deal with inheritance a bit differently.

@Vanuan
Vanuan added a note

It might be not this particular commit, but a series of changes started by this.
If you have class Table < ActiveRecord::Base and class TableChild < Table, than the result of TableChild.generated_attribyte_methods would be different from Table.generated_attribyte_methods

#5516

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ if methods.respond_to?(caster)
+ if @attributes.has_key?(attr_name)
+ @attributes_cache[attr_name] || methods.send(caster, @attributes[attr_name])
+ end
else
_read_attribute attr_name
end
View
11 activerecord/lib/active_record/attribute_methods/serialization.rb
@@ -58,14 +58,11 @@ def serialize(attr_name, class_name = Object)
self.serialized_attributes = serialized_attributes.merge(attr_name.to_s => coder)
end
- def define_method_attribute(attr_name)
+ private
+
+ def attribute_cast_code(attr_name)
if serialized_attributes.include?(attr_name)
- generated_attribute_methods.module_eval(<<-CODE, __FILE__, __LINE__)
- def _#{attr_name}
- @attributes['#{attr_name}'].unserialized_value
- end
- alias #{attr_name} _#{attr_name}
- CODE
+ "v.unserialized_value"
else
super
end
View
26 activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb
@@ -19,18 +19,22 @@ module ClassMethods
# Defined for all +datetime+ and +timestamp+ attributes when +time_zone_aware_attributes+ are enabled.
# This enhanced read method automatically converts the UTC time stored in the database to the time
# zone stored in Time.zone.
- def define_method_attribute(attr_name)
+ def attribute_access_code(attr_name)
if create_time_zone_conversion_attribute?(attr_name, columns_hash[attr_name])
- method_body, line = <<-EOV, __LINE__ + 1
- def _#{attr_name}
- cached = @attributes_cache['#{attr_name}']
- return cached if cached
- time = _read_attribute('#{attr_name}')
- @attributes_cache['#{attr_name}'] = time.acts_like?(:time) ? time.in_time_zone : time
- end
- alias #{attr_name} _#{attr_name}
- EOV
- generated_attribute_methods.module_eval(method_body, __FILE__, line)
+ <<-CODE
+ cached = @attributes_cache['#{attr_name}']
+ return cached if cached
+ time = _read_attribute('#{attr_name}')
+ @attributes_cache['#{attr_name}'] = time.acts_like?(:time) ? time.in_time_zone : time
+ CODE
+ else
+ super
+ end
+ end
+
+ def attribute_cast_code(attr_name)
+ if create_time_zone_conversion_attribute?(attr_name, columns_hash[attr_name])
+ "v.acts_like?(:time) ? v.in_time_zone : v"
else
super
end
@Vanuan

Single table inheritance is broken here.

@jonleighton

I don't think so, because STI always has the same DB columns. However, the code is changed in later commits to deal with inheritance a bit differently.

@Vanuan

It might be not this particular commit, but a series of changes started by this.
If you have class Table < ActiveRecord::Base and class TableChild < Table, than the result of TableChild.generated_attribyte_methods would be different from Table.generated_attribyte_methods

#5516

Please sign in to comment.
Something went wrong with that request. Please try again.