1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

has_attribute? should be aware of attribute aliases

Related to #39495.

For now, `read_attribute`, `write_attribute`, `[]`, `[]=` are aware of
attribute aliases, but `has_attribute?` is not. It will easily miss
attribute alias resolution before using `has_attribute?`, it is very
inconvenient.

I think the inconvenience is not intended, so `has_attribute?` should be
aware of attribute aliases like as others for consistency.
This commit is contained in:
Ryuta Kamizono 2020-06-01 10:24:51 +09:00
parent 8504576bcf
commit c53c418e6f
9 changed files with 59 additions and 20 deletions

View file

@ -315,7 +315,7 @@ module ActiveRecord
# Returns true if record contains the foreign_key # Returns true if record contains the foreign_key
def foreign_key_for?(record) def foreign_key_for?(record)
record.has_attribute?(reflection.foreign_key) record._has_attribute?(reflection.foreign_key.to_s)
end end
# This should be implemented to return the values of the relevant key(s) on the owner, # This should be implemented to return the values of the relevant key(s) on the owner,

View file

@ -172,13 +172,21 @@ module ActiveRecord
# Returns true if the given attribute exists, otherwise false. # Returns true if the given attribute exists, otherwise false.
# #
# class Person < ActiveRecord::Base # class Person < ActiveRecord::Base
# alias_attribute :new_name, :name
# end # end
# #
# Person.has_attribute?('name') # => true # Person.has_attribute?('name') # => true
# Person.has_attribute?(:age) # => true # Person.has_attribute?('new_name') # => true
# Person.has_attribute?(:nothing) # => false # Person.has_attribute?(:age) # => true
# Person.has_attribute?(:nothing) # => false
def has_attribute?(attr_name) def has_attribute?(attr_name)
attribute_types.key?(attr_name.to_s) attr_name = attr_name.to_s
attr_name = attribute_aliases[attr_name] || attr_name
_has_attribute?(attr_name)
end
def _has_attribute?(attr_name) # :nodoc:
attribute_types.key?(attr_name)
end end
# Returns the column object for the named attribute. # Returns the column object for the named attribute.
@ -227,7 +235,7 @@ module ActiveRecord
# have been allocated but not yet initialized. # have been allocated but not yet initialized.
if defined?(@attributes) if defined?(@attributes)
if name = self.class.symbol_column_to_string(name.to_sym) if name = self.class.symbol_column_to_string(name.to_sym)
return has_attribute?(name) return _has_attribute?(name)
end end
end end
@ -237,14 +245,22 @@ module ActiveRecord
# Returns +true+ if the given attribute is in the attributes hash, otherwise +false+. # Returns +true+ if the given attribute is in the attributes hash, otherwise +false+.
# #
# class Person < ActiveRecord::Base # class Person < ActiveRecord::Base
# alias_attribute :new_name, :name
# end # end
# #
# person = Person.new # person = Person.new
# person.has_attribute?(:name) # => true # person.has_attribute?(:name) # => true
# person.has_attribute?('age') # => true # person.has_attribute?(:new_name) # => true
# person.has_attribute?(:nothing) # => false # person.has_attribute?('age') # => true
# person.has_attribute?(:nothing) # => false
def has_attribute?(attr_name) def has_attribute?(attr_name)
@attributes.key?(attr_name.to_s) attr_name = attr_name.to_s
attr_name = self.class.attribute_aliases[attr_name] || attr_name
_has_attribute?(attr_name)
end
def _has_attribute?(attr_name) # :nodoc:
@attributes.key?(attr_name)
end end
# Returns an array of names for the attributes available on this object. # Returns an array of names for the attributes available on this object.

View file

@ -470,7 +470,7 @@ module ActiveRecord
def association_foreign_key_changed?(reflection, record, key) def association_foreign_key_changed?(reflection, record, key)
return false if reflection.through_reflection? return false if reflection.through_reflection?
record.has_attribute?(reflection.foreign_key) && record[reflection.foreign_key] != key record._has_attribute?(reflection.foreign_key.to_s) && record._read_attribute(reflection.foreign_key) != key
end end
# Saves the associated record if it's new or <tt>:autosave</tt> is enabled. # Saves the associated record if it's new or <tt>:autosave</tt> is enabled.

View file

@ -533,7 +533,7 @@ module ActiveRecord
# allocated but not initialized. # allocated but not initialized.
inspection = if defined?(@attributes) && @attributes inspection = if defined?(@attributes) && @attributes
self.class.attribute_names.collect do |name| self.class.attribute_names.collect do |name|
if has_attribute?(name) if _has_attribute?(name)
attr = _read_attribute(name) attr = _read_attribute(name)
value = if attr.nil? value = if attr.nil?
attr.inspect attr.inspect
@ -557,7 +557,7 @@ module ActiveRecord
return super if custom_inspect_method_defined? return super if custom_inspect_method_defined?
pp.object_address_group(self) do pp.object_address_group(self) do
if defined?(@attributes) && @attributes if defined?(@attributes) && @attributes
attr_names = self.class.attribute_names.select { |name| has_attribute?(name) } attr_names = self.class.attribute_names.select { |name| _has_attribute?(name) }
pp.seplist(attr_names, proc { pp.text "," }) do |attr_name| pp.seplist(attr_names, proc { pp.text "," }) do |attr_name|
pp.breakable " " pp.breakable " "
pp.group(1) do pp.group(1) do

View file

@ -52,7 +52,7 @@ module ActiveRecord
raise NotImplementedError, "#{self} is an abstract class and cannot be instantiated." raise NotImplementedError, "#{self} is an abstract class and cannot be instantiated."
end end
if has_attribute?(inheritance_column) if _has_attribute?(inheritance_column)
subclass = subclass_from_attributes(attributes) subclass = subclass_from_attributes(attributes)
if subclass.nil? && scope_attributes = current_scope&.scope_for_create if subclass.nil? && scope_attributes = current_scope&.scope_for_create
@ -245,7 +245,7 @@ module ActiveRecord
end end
def using_single_table_inheritance?(record) def using_single_table_inheritance?(record)
record[inheritance_column].present? && has_attribute?(inheritance_column) record[inheritance_column].present? && _has_attribute?(inheritance_column)
end end
def find_sti_class(type_name) def find_sti_class(type_name)

View file

@ -97,16 +97,14 @@ module ActiveRecord
def cache_version def cache_version
return unless cache_versioning return unless cache_versioning
timestamp_column = self.class.attribute_aliases["updated_at"] || "updated_at" if has_attribute?("updated_at")
if has_attribute?(timestamp_column)
timestamp = updated_at_before_type_cast timestamp = updated_at_before_type_cast
if can_use_fast_cache_version?(timestamp) if can_use_fast_cache_version?(timestamp)
raw_timestamp_to_cache_version(timestamp) raw_timestamp_to_cache_version(timestamp)
elsif timestamp = updated_at elsif timestamp = updated_at
timestamp.utc.to_s(cache_timestamp_format) timestamp.utc.to_s(cache_timestamp_format)
end end
elsif self.class.has_attribute?(timestamp_column) elsif self.class.has_attribute?("updated_at")
raise ActiveModel::MissingAttributeError, "missing attribute: updated_at" raise ActiveModel::MissingAttributeError, "missing attribute: updated_at"
end end
end end

View file

@ -11,7 +11,7 @@ module ActiveRecord #:nodoc:
end end
def serializable_hash(options = nil) def serializable_hash(options = nil)
if self.class.has_attribute?(self.class.inheritance_column) if self.class._has_attribute?(self.class.inheritance_column)
options = options ? options.dup : {} options = options ? options.dup : {}
options[:except] = Array(options[:except]).map(&:to_s) options[:except] = Array(options[:except]).map(&:to_s)

View file

@ -1277,14 +1277,38 @@ class BasicsTest < ActiveRecord::TestCase
assert Company.has_attribute?("id") assert Company.has_attribute?("id")
assert Company.has_attribute?("type") assert Company.has_attribute?("type")
assert Company.has_attribute?("name") assert Company.has_attribute?("name")
assert Company.has_attribute?("new_name")
assert Company.has_attribute?("metadata") assert Company.has_attribute?("metadata")
assert_not Company.has_attribute?("lastname") assert_not Company.has_attribute?("lastname")
assert_not Company.has_attribute?("age") assert_not Company.has_attribute?("age")
company = Company.new
assert company.has_attribute?("id")
assert company.has_attribute?("type")
assert company.has_attribute?("name")
assert company.has_attribute?("new_name")
assert company.has_attribute?("metadata")
assert_not company.has_attribute?("lastname")
assert_not company.has_attribute?("age")
end end
def test_has_attribute_with_symbol def test_has_attribute_with_symbol
assert Company.has_attribute?(:id) assert Company.has_attribute?(:id)
assert Company.has_attribute?(:type)
assert Company.has_attribute?(:name)
assert Company.has_attribute?(:new_name)
assert Company.has_attribute?(:metadata)
assert_not Company.has_attribute?(:lastname)
assert_not Company.has_attribute?(:age) assert_not Company.has_attribute?(:age)
company = Company.new
assert company.has_attribute?(:id)
assert company.has_attribute?(:type)
assert company.has_attribute?(:name)
assert company.has_attribute?(:new_name)
assert company.has_attribute?(:metadata)
assert_not company.has_attribute?(:lastname)
assert_not company.has_attribute?(:age)
end end
def test_attribute_names_on_table_not_exists def test_attribute_names_on_table_not_exists

View file

@ -14,6 +14,7 @@ class Company < AbstractCompany
has_many :contracts has_many :contracts
has_many :developers, through: :contracts has_many :developers, through: :contracts
alias_attribute :new_name, :name
attribute :metadata, :json attribute :metadata, :json
scope :of_first_firm, lambda { scope :of_first_firm, lambda {