Skip to content

Commit a0993f8

Browse files
committed
Define missing attribute methods from method_missing
Ref: mastodon/mastodon#27622 If applications don't eager load and use the old Rails 6.1 Marshal format, they can load Active Record instances from caches without calling `init_internals` hence attribute methods and aliases are nnot defined yet, leading to `NoMethodError` Initially I was hopping to fully get rid of the `define_attribute_methods` call in `init_internals` in favor of this new method missing, as it would remove work from the happy path, unfortunately that isn't possible because if a generated method overrides a default method inherited from object, `method_missing` won't be called. e.g. `Kernel#format` We may want to get rid of this extra code once we remove support for the 6.1 marshal format.
1 parent 1702b6c commit a0993f8

File tree

6 files changed

+74
-30
lines changed

6 files changed

+74
-30
lines changed

activerecord/lib/active_record/attribute_methods.rb

Lines changed: 55 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -63,24 +63,6 @@ def eagerly_generate_alias_attribute_methods(_new_name, _old_name) # :nodoc:
6363
# alias attributes in Active Record are lazily generated
6464
end
6565

66-
def generate_alias_attributes # :nodoc:
67-
superclass.generate_alias_attributes unless superclass == Base
68-
return if @alias_attributes_mass_generated
69-
70-
GeneratedAttributeMethods::LOCK.synchronize do
71-
return if @alias_attributes_mass_generated
72-
ActiveSupport::CodeGenerator.batch(generated_attribute_methods, __FILE__, __LINE__) do |code_generator|
73-
aliases_by_attribute_name.each do |old_name, new_names|
74-
new_names.each do |new_name|
75-
generate_alias_attribute_methods(code_generator, new_name, old_name)
76-
end
77-
end
78-
end
79-
80-
@alias_attributes_mass_generated = true
81-
end
82-
end
83-
8466
def alias_attribute_method_definition(code_generator, pattern, new_name, old_name)
8567
method_name = pattern.method_name(new_name).to_s
8668
target_name = pattern.method_name(old_name).to_s
@@ -120,6 +102,10 @@ def alias_attribute_method_definition(code_generator, pattern, new_name, old_nam
120102
end
121103
end
122104

105+
def attribute_methods_generated? # :nodoc:
106+
@attribute_methods_generated
107+
end
108+
123109
# Generates all the attribute related methods for columns in the database
124110
# accessors, mutators and query methods.
125111
def define_attribute_methods # :nodoc:
@@ -128,11 +114,29 @@ def define_attribute_methods # :nodoc:
128114
# attribute methods.
129115
GeneratedAttributeMethods::LOCK.synchronize do
130116
return false if @attribute_methods_generated
131-
superclass.define_attribute_methods unless base_class?
132-
super(attribute_names)
133-
alias_attribute(:id_value, :id) if attribute_names.include?("id")
117+
118+
superclass.define_attribute_methods unless superclass == Base
119+
120+
unless abstract_class?
121+
load_schema
122+
super(attribute_names)
123+
if _has_attribute?("id")
124+
alias_attribute(:id_value, :id)
125+
end
126+
end
127+
128+
ActiveSupport::CodeGenerator.batch(generated_attribute_methods, __FILE__, __LINE__) do |code_generator|
129+
aliases_by_attribute_name.each do |old_name, new_names|
130+
new_names.each do |new_name|
131+
generate_alias_attribute_methods(code_generator, new_name, old_name)
132+
end
133+
end
134+
end
135+
134136
@attribute_methods_generated = true
137+
@alias_attributes_mass_generated = true
135138
end
139+
true
136140
end
137141

138142
def undefine_attribute_methods # :nodoc:
@@ -459,6 +463,36 @@ def accessed_fields
459463
end
460464

461465
private
466+
def respond_to_missing?(name, include_private = false)
467+
if self.class.define_attribute_methods
468+
# Some methods weren't defined yet.
469+
return true if self.class.method_defined?(name)
470+
return true if include_private && self.class.private_method_defined?(name)
471+
end
472+
473+
super
474+
end
475+
476+
def method_missing(name, ...)
477+
unless self.class.attribute_methods_generated?
478+
if self.class.method_defined?(name)
479+
# The method is explicitly defined in the model, but calls a generated
480+
# method with super. So we must resume the call chain at the right setp.
481+
last_method = method(name)
482+
last_method = last_method.super_method while last_method.super_method
483+
self.class.define_attribute_methods
484+
if last_method.super_method
485+
return last_method.super_method.call(...)
486+
end
487+
elsif self.class.define_attribute_methods
488+
# Some attribute methods weren't generated yet, we retry the call
489+
return public_send(name, ...)
490+
end
491+
end
492+
493+
super
494+
end
495+
462496
def attribute_method?(attr_name)
463497
# We check defined? because Syck calls respond_to? before actually calling initialize.
464498
defined?(@attributes) && @attributes.key?(attr_name)

activerecord/lib/active_record/core.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -769,7 +769,6 @@ def init_internals
769769
@strict_loading_mode = :all
770770

771771
klass.define_attribute_methods
772-
klass.generate_alias_attributes
773772
end
774773

775774
def initialize_internals_callback

activerecord/lib/active_record/model_schema.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -422,12 +422,12 @@ def attributes_builder # :nodoc:
422422
end
423423

424424
def columns_hash # :nodoc:
425-
load_schema
425+
load_schema unless @columns_hash
426426
@columns_hash
427427
end
428428

429429
def columns
430-
load_schema
430+
load_schema unless @columns
431431
@columns ||= columns_hash.values.freeze
432432
end
433433

@@ -526,7 +526,7 @@ def reset_column_information
526526
def load_schema # :nodoc:
527527
return if schema_loaded?
528528
@load_schema_monitor.synchronize do
529-
return if @columns_hash
529+
return if schema_loaded?
530530

531531
load_schema!
532532

activerecord/test/cases/attribute_methods/read_test.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,19 @@ def setup
1414
@klass = Class.new(Class.new { def self.initialize_generated_modules; end }) do
1515
def self.superclass; Base; end
1616
def self.base_class?; true; end
17+
def self.abstract_class?; false; end
18+
def self.load_schema; end
1719

1820
include ActiveRecord::AttributeMethods
1921

2022
def self.attribute_names
2123
%w{ one two three }
2224
end
2325

26+
def self.attribute_types
27+
{}
28+
end
29+
2430
def self.primary_key
2531
end
2632

activerecord/test/cases/attribute_methods_test.rb

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,11 +1034,16 @@ def name
10341034

10351035
topic = topic_class.new(title: "New topic")
10361036
assert_equal("New topic", topic.subject_to_be_undefined)
1037+
assert_equal true, topic_class.method_defined?(:subject_to_be_undefined)
10371038
topic_class.undefine_attribute_methods
1039+
assert_equal false, topic_class.method_defined?(:subject_to_be_undefined)
10381040

1039-
assert_raises(NoMethodError, match: /undefined method `subject_to_be_undefined'/) do
1040-
topic.subject_to_be_undefined
1041-
end
1041+
topic.subject_to_be_undefined
1042+
assert_equal true, topic_class.method_defined?(:subject_to_be_undefined)
1043+
1044+
topic_class.undefine_attribute_methods
1045+
assert_equal true, topic.respond_to?(:subject_to_be_undefined)
1046+
assert_equal true, topic_class.method_defined?(:subject_to_be_undefined)
10421047
end
10431048

10441049
test "#define_attribute_methods brings back undefined aliases" do
@@ -1052,11 +1057,11 @@ def name
10521057
assert_equal("New topic", topic.title_alias_to_be_undefined)
10531058
topic_class.undefine_attribute_methods
10541059

1055-
assert_not_respond_to topic, :title_alias_to_be_undefined
1060+
assert_equal false, topic_class.method_defined?(:title_alias_to_be_undefined)
10561061

10571062
topic_class.define_attribute_methods
10581063

1059-
assert_respond_to topic, :title_alias_to_be_undefined
1064+
assert_equal true, topic_class.method_defined?(:title_alias_to_be_undefined)
10601065
assert_equal "New topic", topic.title_alias_to_be_undefined
10611066
end
10621067

0 commit comments

Comments
 (0)