Skip to content

Commit

Permalink
Rename attribute related instance variables to better express intent
Browse files Browse the repository at this point in the history
`@attributes` was actually used for `_before_type_cast` and friends,
while `@attributes_cache` is the type cast version (and caching is the
wrong word there, but I'm working on removing the conditionals around
that). I opted for `@raw_attributes`, because `_before_type_cast` is
also semantically misleading. The values in said hash are in the state
given by the form builder or database, so raw seemed to be a good word.
  • Loading branch information
sgrif committed May 30, 2014
1 parent 8c77b0a commit eb6cee9
Show file tree
Hide file tree
Showing 13 changed files with 44 additions and 44 deletions.
10 changes: 5 additions & 5 deletions activerecord/lib/active_record/attribute_methods.rb
Expand Up @@ -239,9 +239,9 @@ def respond_to?(name, include_private = false)

# If the result is true then check for the select case.
# For queries selecting a subset of columns, return false for unselected columns.
# We check defined?(@attributes) not to issue warnings if called on objects that
# We check defined?(@raw_attributes) not to issue warnings if called on objects that
# have been allocated but not yet initialized.
if defined?(@attributes) && @attributes.any? && self.class.column_names.include?(name)
if defined?(@raw_attributes) && @raw_attributes.any? && self.class.column_names.include?(name)
return has_attribute?(name)
end

Expand All @@ -258,7 +258,7 @@ def respond_to?(name, include_private = false)
# person.has_attribute?('age') # => true
# person.has_attribute?(:nothing) # => false
def has_attribute?(attr_name)
@attributes.has_key?(attr_name.to_s)
@raw_attributes.has_key?(attr_name.to_s)
end

# Returns an array of names for the attributes available on this object.
Expand All @@ -270,7 +270,7 @@ def has_attribute?(attr_name)
# person.attribute_names
# # => ["id", "created_at", "updated_at", "name", "age"]
def attribute_names
@attributes.keys
@raw_attributes.keys
end

# Returns a hash of all the attributes with their names as keys and the values of the attributes as values.
Expand Down Expand Up @@ -424,7 +424,7 @@ def arel_attributes_with_values_for_update(attribute_names) # :nodoc:

def attribute_method?(attr_name) # :nodoc:
# We check defined? because Syck calls respond_to? before actually calling initialize.
defined?(@attributes) && @attributes.include?(attr_name)
defined?(@raw_attributes) && @raw_attributes.include?(attr_name)
end

private
Expand Down
Expand Up @@ -43,7 +43,7 @@ module BeforeTypeCast
# task.read_attribute_before_type_cast('completed_on') # => "2012-10-21"
# task.read_attribute_before_type_cast(:completed_on) # => "2012-10-21"
def read_attribute_before_type_cast(attr_name)
@attributes[attr_name.to_s]
@raw_attributes[attr_name.to_s]
end

# Returns a hash of attributes before typecasting and deserialization.
Expand All @@ -57,7 +57,7 @@ def read_attribute_before_type_cast(attr_name)
# task.attributes_before_type_cast
# # => {"id"=>nil, "title"=>nil, "is_done"=>true, "completed_on"=>"2012-10-21", "created_at"=>nil, "updated_at"=>nil}
def attributes_before_type_cast
@attributes
@raw_attributes
end

private
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/attribute_methods/dirty.rb
Expand Up @@ -55,7 +55,7 @@ def init_changed_attributes
# optimistic locking) won't get written unless they get marked as changed
self.class.columns.each do |c|
attr, orig_value = c.name, c.default
changed_attributes[attr] = orig_value if _field_changed?(attr, orig_value, @attributes[attr])
changed_attributes[attr] = orig_value if _field_changed?(attr, orig_value, @raw_attributes[attr])
end
end

Expand Down
10 changes: 5 additions & 5 deletions activerecord/lib/active_record/attribute_methods/read.rb
Expand Up @@ -22,7 +22,7 @@ module Read
# the attribute name. Using a constant means that we do not have
# to allocate an object on each call to the attribute method.
# Making it frozen means that it doesn't get duped when used to
# key the @attributes_cache in read_attribute.
# key the @attributes in read_attribute.
def method_body(method_name, const_name)
<<-EOMETHOD
def #{method_name}
Expand Down Expand Up @@ -108,22 +108,22 @@ def read_attribute(attr_name)
# If it's cached, just return it
# We use #[] first as a perf optimization for non-nil values. See https://gist.github.com/jonleighton/3552829.
name = attr_name.to_s
@attributes_cache[name] || @attributes_cache.fetch(name) {
@attributes[name] || @attributes.fetch(name) {
column = @column_types_override[name] if @column_types_override
column ||= @column_types[name]

return @attributes.fetch(name) {
return @raw_attributes.fetch(name) {
if name == 'id' && self.class.primary_key != name
read_attribute(self.class.primary_key)
end
} unless column

value = @attributes.fetch(name) {
value = @raw_attributes.fetch(name) {
return block_given? ? yield(name) : nil
}

if self.class.cache_attribute?(name)
@attributes_cache[name] = column.type_cast(value)
@attributes[name] = column.type_cast(value)
else
column.type_cast value
end
Expand Down
Expand Up @@ -127,7 +127,7 @@ def attributes_before_type_cast

def typecasted_attribute_value(name)
if self.class.serialized_attributes.include?(name)
@attributes[name].serialized_value
@raw_attributes[name].serialized_value
else
super
end
Expand All @@ -136,7 +136,7 @@ def typecasted_attribute_value(name)
def attributes_for_coder
attribute_names.each_with_object({}) do |name, attrs|
attrs[name] = if self.class.serialized_attributes.include?(name)
@attributes[name].serialized_value
@raw_attributes[name].serialized_value
else
read_attribute(name)
end
Expand Down
Expand Up @@ -36,7 +36,7 @@ def #{attr_name}=(time)
previous_time = attribute_changed?("#{attr_name}") ? changed_attributes["#{attr_name}"] : read_attribute(:#{attr_name})
write_attribute(:#{attr_name}, time)
#{attr_name}_will_change! if previous_time != time_with_zone
@attributes_cache["#{attr_name}"] = time_with_zone
@attributes["#{attr_name}"] = time_with_zone
end
EOV
generated_attribute_methods.module_eval(method_body, __FILE__, line)
Expand Down
10 changes: 5 additions & 5 deletions activerecord/lib/active_record/attribute_methods/write.rb
Expand Up @@ -69,19 +69,19 @@ def attribute=(attribute_name, value)
def write_attribute_with_type_cast(attr_name, value, type_cast_method)
attr_name = attr_name.to_s
attr_name = self.class.primary_key if attr_name == 'id' && self.class.primary_key
@attributes_cache.delete(attr_name)
@attributes.delete(attr_name)
column = column_for_attribute(attr_name)

# If we're dealing with a binary column, write the data to the cache
# so we don't attempt to typecast multiple times.
if column && column.binary?
@attributes_cache[attr_name] = value
@attributes[attr_name] = value
end

if column
@attributes[attr_name] = column.public_send(type_cast_method, value)
elsif @attributes.has_key?(attr_name)
@attributes[attr_name] = value
@raw_attributes[attr_name] = column.public_send(type_cast_method, value)
elsif @raw_attributes.has_key?(attr_name)
@raw_attributes[attr_name] = value
else
raise ActiveModel::MissingAttributeError, "can't write unknown attribute `#{attr_name}'"
end
Expand Down
22 changes: 11 additions & 11 deletions activerecord/lib/active_record/core.rb
Expand Up @@ -252,7 +252,7 @@ def initialize(attributes = nil, options = {})
defaults = self.class.column_defaults.dup
defaults.each { |k, v| defaults[k] = v.dup if v.duplicable? }

@attributes = self.class.initialize_attributes(defaults)
@raw_attributes = self.class.initialize_attributes(defaults)
@column_types_override = nil
@column_types = self.class.column_types

Expand All @@ -278,7 +278,7 @@ def initialize(attributes = nil, options = {})
# post.init_with('attributes' => { 'title' => 'hello world' })
# post.title # => 'hello world'
def init_with(coder)
@attributes = self.class.initialize_attributes(coder['attributes'])
@raw_attributes = self.class.initialize_attributes(coder['attributes'])
@column_types_override = coder['column_types']
@column_types = self.class.column_types

Expand Down Expand Up @@ -325,14 +325,14 @@ def initialize_dup(other) # :nodoc:
cloned_attributes = other.clone_attributes(:read_attribute_before_type_cast)
self.class.initialize_attributes(cloned_attributes, :serialized => false)

@attributes = cloned_attributes
@attributes[self.class.primary_key] = nil
@raw_attributes = cloned_attributes
@raw_attributes[self.class.primary_key] = nil

run_callbacks(:initialize) unless _initialize_callbacks.empty?

@aggregation_cache = {}
@association_cache = {}
@attributes_cache = {}
@attributes = {}

@new_record = true
@destroyed = false
Expand Down Expand Up @@ -383,13 +383,13 @@ def hash
# accessible, even on destroyed records, but cloned models will not be
# frozen.
def freeze
@attributes = @attributes.clone.freeze
@raw_attributes = @raw_attributes.clone.freeze
self
end

# Returns +true+ if the attributes hash has been frozen.
def frozen?
@attributes.frozen?
@raw_attributes.frozen?
end

# Allows sort on objects
Expand Down Expand Up @@ -418,9 +418,9 @@ def connection_handler

# Returns the contents of the record as a nicely formatted string.
def inspect
# We check defined?(@attributes) not to issue warnings if the object is
# We check defined?(@raw_attributes) not to issue warnings if the object is
# allocated but not initialized.
inspection = if defined?(@attributes) && @attributes
inspection = if defined?(@raw_attributes) && @raw_attributes
self.class.column_names.collect { |name|
if has_attribute?(name)
"#{name}: #{attribute_for_inspect(name)}"
Expand Down Expand Up @@ -496,11 +496,11 @@ def to_ary # :nodoc:

def init_internals
pk = self.class.primary_key
@attributes[pk] = nil unless @attributes.key?(pk)
@raw_attributes[pk] = nil unless @raw_attributes.key?(pk)

@aggregation_cache = {}
@association_cache = {}
@attributes_cache = {}
@attributes = {}
@readonly = false
@destroyed = false
@marked_for_destruction = false
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/locking/optimistic.rb
Expand Up @@ -66,7 +66,7 @@ def increment_lock
send(lock_col + '=', previous_lock_value + 1)
end

def _update_record(attribute_names = @attributes.keys) #:nodoc:
def _update_record(attribute_names = @raw_attributes.keys) #:nodoc:
return super unless locking_enabled?
return 0 if attribute_names.empty?

Expand Down
10 changes: 5 additions & 5 deletions activerecord/lib/active_record/persistence.rb
Expand Up @@ -179,8 +179,8 @@ def destroy!
# So any change to the attributes in either instance will affect the other.
def becomes(klass)
became = klass.new
became.instance_variable_set("@raw_attributes", @raw_attributes)
became.instance_variable_set("@attributes", @attributes)
became.instance_variable_set("@attributes_cache", @attributes_cache)
became.instance_variable_set("@changed_attributes", @changed_attributes) if defined?(@changed_attributes)
became.instance_variable_set("@new_record", new_record?)
became.instance_variable_set("@destroyed", destroyed?)
Expand Down Expand Up @@ -396,11 +396,11 @@ def reload(options = nil)
self.class.unscoped { self.class.find(id) }
end

@attributes.update(fresh_object.instance_variable_get('@attributes'))
@raw_attributes.update(fresh_object.instance_variable_get('@raw_attributes'))

@column_types = self.class.column_types
@column_types_override = fresh_object.instance_variable_get('@column_types_override')
@attributes_cache = {}
@attributes = {}
self
end

Expand Down Expand Up @@ -490,7 +490,7 @@ def create_or_update

# Updates the associated record with values matching those of the instance attributes.
# Returns the number of affected rows.
def _update_record(attribute_names = @attributes.keys)
def _update_record(attribute_names = @raw_attributes.keys)
attributes_values = arel_attributes_with_values_for_update(attribute_names)
if attributes_values.empty?
0
Expand All @@ -501,7 +501,7 @@ def _update_record(attribute_names = @attributes.keys)

# Creates a record with values matching those of the instance attributes
# and returns its id.
def _create_record(attribute_names = @attributes.keys)
def _create_record(attribute_names = @raw_attributes.keys)
attributes_values = arel_attributes_with_values_for_create(attribute_names)

new_id = self.class.unscoped.insert attributes_values
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/result.rb
Expand Up @@ -95,7 +95,7 @@ def hash_rows
@hash_rows ||=
begin
# We freeze the strings to prevent them getting duped when
# used as keys in ActiveRecord::Base's @attributes hash
# used as keys in ActiveRecord::Base's @raw_attributes hash
columns = @columns.map { |c| c.dup.freeze }
@rows.map { |row|
# In the past we used Hash[columns.zip(row)]
Expand Down
8 changes: 4 additions & 4 deletions activerecord/lib/active_record/transactions.rb
Expand Up @@ -347,7 +347,7 @@ def remember_transaction_record_state #:nodoc:
@_start_transaction_state[:destroyed] = @destroyed
end
@_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) + 1
@_start_transaction_state[:frozen?] = @attributes.frozen?
@_start_transaction_state[:frozen?] = @raw_attributes.frozen?
end

# Clear the new record state and id of a record.
Expand All @@ -368,16 +368,16 @@ def restore_transaction_record_state(force = false) #:nodoc:
if transaction_level < 1 || force
restore_state = @_start_transaction_state
was_frozen = restore_state[:frozen?]
@attributes = @attributes.dup if @attributes.frozen?
@raw_attributes = @raw_attributes.dup if @raw_attributes.frozen?
@new_record = restore_state[:new_record]
@destroyed = restore_state[:destroyed]
if restore_state.has_key?(:id)
write_attribute(self.class.primary_key, restore_state[:id])
else
@raw_attributes.delete(self.class.primary_key)
@attributes.delete(self.class.primary_key)
@attributes_cache.delete(self.class.primary_key)
end
@attributes.freeze if was_frozen
@raw_attributes.freeze if was_frozen
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/attribute_methods_test.rb
Expand Up @@ -534,7 +534,7 @@ def test_cacheable_columns_are_actually_cached

def test_accessing_cached_attributes_caches_the_converted_values_and_nothing_else
t = topics(:first)
cache = t.instance_variable_get "@attributes_cache"
cache = t.instance_variable_get "@attributes"

assert_not_nil cache
assert cache.empty?
Expand Down

0 comments on commit eb6cee9

Please sign in to comment.