From 403743ed88dd92770faa96334de877c392547f21 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 18 Jun 2024 09:37:16 +0200 Subject: [PATCH] Fix `alias_attribute` to ignore methods defined in parent classes Fix: https://github.com/rails/rails/issues/52144 When defining regular attributes, inherited methods aren't overriden, however when defining aliased attributes, inherited methods aren't considered. This behavior could be debatted, but that was the behavior prior to https://github.com/rails/rails/pull/52118, so I'm restoring it. --- .../lib/active_model/attribute_methods.rb | 39 +++++++++++-------- .../lib/active_record/attribute_methods.rb | 2 +- .../test/cases/attribute_methods_test.rb | 19 +++++++++ 3 files changed, 43 insertions(+), 17 deletions(-) diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index e5c51eac61..521907a443 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -312,26 +312,33 @@ def define_attribute_method(attr_name, _owner: generated_attribute_methods, as: end end - def define_attribute_method_pattern(pattern, attr_name, owner:, as:) # :nodoc: + def define_attribute_method_pattern(pattern, attr_name, owner:, as:, override: false) # :nodoc: canonical_method_name = pattern.method_name(attr_name) public_method_name = pattern.method_name(as) - unless instance_method_already_implemented?(public_method_name) - generate_method = "define_method_#{pattern.proxy_target}" + # If defining a regular attribute method, we don't override methods that are explictly + # defined in parrent classes. + if instance_method_already_implemented?(public_method_name) + # However, for `alias_attribute`, we always define the method. + # We check for override second because `instance_method_already_implemented?` + # also check for dangerous methods. + return unless override + end - if respond_to?(generate_method, true) - send(generate_method, attr_name.to_s, owner: owner, as: as) - else - define_proxy_call( - owner, - canonical_method_name, - pattern.proxy_target, - pattern.parameters, - attr_name.to_s, - namespace: :active_model_proxy, - as: public_method_name - ) - end + generate_method = "define_method_#{pattern.proxy_target}" + + if respond_to?(generate_method, true) + send(generate_method, attr_name.to_s, owner: owner, as: as) + else + define_proxy_call( + owner, + canonical_method_name, + pattern.proxy_target, + pattern.parameters, + attr_name.to_s, + namespace: :active_model_proxy, + as: public_method_name + ) end end diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index db47ec83b3..0aec63f057 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -91,7 +91,7 @@ def alias_attribute_method_definition(code_generator, pattern, new_name, old_nam raise ArgumentError, "#{self.name} model aliases `#{old_name}`, but `#{old_name}` is not an attribute. " \ "Use `alias_method :#{new_name}, :#{old_name}` or define the method manually." else - define_attribute_method_pattern(pattern, old_name, owner: code_generator, as: new_name) + define_attribute_method_pattern(pattern, old_name, owner: code_generator, as: new_name, override: true) end end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index ce529cfd33..1910f896cd 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -1217,6 +1217,25 @@ def some_method_that_is_not_on_super alias_attribute :subject, :title end + test "#alias_attribute override methods defined in parent models" do + parent_model = Class.new(ActiveRecord::Base) do + self.abstract_class = true + + def subject + "Abstract Subject" + end + end + + subclass = Class.new(parent_model) do + self.table_name = "topics" + alias_attribute :subject, :title + end + + obj = subclass.new + obj.title = "hey" + assert_equal("hey", obj.subject) + end + test "aliases to the same attribute name do not conflict with each other" do first_model_object = ToBeLoadedFirst.new(author_name: "author 1") assert_equal("author 1", first_model_object.subject)